Migrating to apache commons configuration 1.10 for parsing ETL jobs c…

…onfigurations.
datahub-project · Nov 10, 2017 · 5d84d37 · 5d84d37
1 parent 668e631
commit 5d84d37
Show file tree

Hide file tree

Showing 4 changed files with 93 additions and 38 deletions.
diff --git a/gradle/scripts/dependency.gradle b/gradle/scripts/dependency.gradle
@@ -19,6 +19,7 @@ ext.externalDependency = [
     "jsoup"               : "org.jsoup:jsoup:1.8.3",
     "commons_io"          : "commons-io:commons-io:2.5",
     "commons_lang3"       : "org.apache.commons:commons-lang3:3.6",
+    "commons_config"      : "commons-configuration:commons-configuration:1.10",
 
     "jackson_databind"    : "com.fasterxml.jackson.core:jackson-databind:2.8.9",
     "jackson_core"        : "com.fasterxml.jackson.core:jackson-core:2.8.9",

diff --git a/wherehows-common/build.gradle b/wherehows-common/build.gradle
@@ -10,6 +10,7 @@ dependencies {
   compile externalDependency.jackson_databind
   compile externalDependency.guava
   compile externalDependency.lombok
+  compile externalDependency.commons_config
 
   testCompile externalDependency.testng
 }

diff --git a/wherehows-common/src/main/java/wherehows/common/utils/JobsUtil.java b/wherehows-common/src/main/java/wherehows/common/utils/JobsUtil.java
@@ -17,45 +17,32 @@
 import java.io.BufferedReader;
 import java.io.File;
 import java.io.IOException;
+import java.nio.charset.Charset;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.HashMap;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Properties;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import org.apache.commons.configuration.Configuration;
+import org.apache.commons.configuration.ConfigurationException;
+import org.apache.commons.configuration.PropertiesConfiguration;
+import org.apache.commons.lang.StringUtils;
 import wherehows.common.Constant;
 
 
 public class JobsUtil {
 
   // Patterns for environmental variables resolution in jobs file.
-  private static final List<Pattern> ENV_VAR_PATTERNS = ImmutableList.<Pattern>builder()
-      .add(Pattern.compile("\\$(.+)")) // $ENV_VAR
-      .add(Pattern.compile("\\$\\{(.+)\\}")) // ${ENV_VAR}
-      .build();
-
-  /**
-   * Reads {@link Properties} from the given file and resolves all environmental variables denoted by ${ENV_VAR_NAME}.
-   *
-   * @param jobFile Path to the property file.
-   * @return Resolved {@link Properties} or null if failed to read from file.
-   */
-  public static Properties getResolvedProperties(Path jobFile) {
-    Properties prop = new Properties();
-    try (BufferedReader reader = Files.newBufferedReader(jobFile)) {
-      prop.load(reader);
-    } catch (IOException ex) {
-      return null;
-    }
-
-    for (Map.Entry<Object, Object> entry : prop.entrySet()) {
-      String value = (String) entry.getValue();
-      prop.setProperty((String) entry.getKey(), resolveEnviornmentalVariable(value));
-    }
-    return prop;
-  }
+  private static final List<Pattern> ENV_VAR_PATTERNS =
+      ImmutableList.<Pattern>builder().add(Pattern.compile("\\$(.+)")) // $ENV_VAR
+          .add(Pattern.compile("\\$\\{(.+)\\}")) // ${ENV_VAR}
+          .build();
 
   /**
    * Resolves the value to the corresponding environmental variable if possible, otherwise returns the original value.
@@ -103,11 +90,11 @@ public static String jobNameFromFileName(String filename) {
   /**
    * Returns a map of job name to job properties for all scheduled and enabled jobs.
    */
-  public static Map<String, Properties> getScheduledJobs(String dir) {
+  public static Map<String, Properties> getScheduledJobs(String dir) throws ConfigurationException {
     Map<String, Properties> jobs = new HashMap<>();
     for (File file : new File(dir).listFiles()) {
       if (file.getAbsolutePath().endsWith(".job")) {
-        Properties prop = getResolvedProperties(file.toPath());
+        Properties prop = getJobConfigProperties(file);
         if (!prop.containsKey(Constant.JOB_DISABLED_KEY) && prop.containsKey(Constant.JOB_CRON_EXPR_KEY)) {
           // job name = file name without the extension.
           jobs.put(jobNameFromFile(file), prop);
@@ -120,11 +107,12 @@ public static Map<String, Properties> getScheduledJobs(String dir) {
   /**
    * Returns a map of job name to job properties which are enabled.
    */
-  public static Map<String, Properties> getEnabledJobs(String dir) {
+  public static Map<String, Properties> getEnabledJobs(String dir) throws ConfigurationException {
     Map<String, Properties> jobs = new HashMap<>();
     for (File file : new File(dir).listFiles()) {
       if (file.getAbsolutePath().endsWith(".job")) {
-        Properties prop = getResolvedProperties(file.toPath());
+        Configuration jobParam = new PropertiesConfiguration(file.getAbsolutePath());
+        Properties prop = getJobConfigProperties(file);
         if (!prop.containsKey(Constant.JOB_DISABLED_KEY)) {
           // job name = file name without the extension.
           jobs.put(jobNameFromFile(file), prop);
@@ -137,17 +125,57 @@ public static Map<String, Properties> getEnabledJobs(String dir) {
   /**
    * Returns a map of job name to job properties which are enabled and of certain type.
    */
-  public static Map<String, Properties> getEnabledJobsByType(String dir, String type) {
+  public static Map<String, Properties> getEnabledJobsByType(String dir, String type) throws ConfigurationException {
     Map<String, Properties> jobs = new HashMap<>();
     for (File file : new File(dir).listFiles()) {
       if (file.getAbsolutePath().endsWith(".job")) {
-        Properties prop = getResolvedProperties(file.toPath());
+        Properties prop = getJobConfigProperties(file);
         if (!prop.containsKey(Constant.JOB_DISABLED_KEY) && prop.getProperty(Constant.JOB_TYPE_KEY, "").equals(type)) {
           // job name = file name without the extension.
           jobs.put(jobNameFromFile(file), prop);
         }
       }
     }
+
     return jobs;
   }
+
+  public static Properties getJobConfigProperties(File jobConfigFile) throws ConfigurationException {
+    Properties prop = new Properties();
+    String propValues = "";
+
+    Configuration jobParam = new PropertiesConfiguration(jobConfigFile.getAbsolutePath());
+
+    if (jobParam != null) {
+      Iterator<String> keyit = jobParam.getKeys();
+      while (keyit.hasNext()) {
+        String key = keyit.next();
+        if (key != null) {
+          Object value = jobParam.getProperty(key);
+          if (value != null && value instanceof String[]) {
+            propValues = String.join(",", (String[]) value);
+            prop.setProperty(key, splitAndResolveEnvironmentalValues(propValues));
+          } else if (value != null && value instanceof List<?>) {
+            propValues = String.join(",", (List<String>) value);
+            prop.setProperty(key, splitAndResolveEnvironmentalValues(propValues));
+          } else if (value != null) {
+            prop.setProperty(key, resolveEnviornmentalVariable(value.toString()));
+          }
+        }
+      }
+    }
+    return prop;
+  }
+
+  public static String splitAndResolveEnvironmentalValues(String propValues) {
+
+    String[] values = propValues.split(",");
+    String commaValues = "";
+    String indexValue = "";
+    for (int i = 0; i < values.length; i++) {
+      indexValue = resolveEnviornmentalVariable(values[i]);
+      values[i] = indexValue;
+    }
+    return String.join(",", (String[]) values);
+  }
 }
diff --git a/wherehows-common/src/test/java/wherehows/common/utils/JobsUtilTest.java b/wherehows-common/src/test/java/wherehows/common/utils/JobsUtilTest.java
@@ -20,6 +20,7 @@
 import java.nio.file.Path;
 import java.util.Map;
 import java.util.Properties;
+import org.apache.commons.configuration.ConfigurationException;
 import org.testng.Assert;
 import org.testng.annotations.Test;
 import wherehows.common.Constant;
@@ -30,23 +31,25 @@
 public class JobsUtilTest {
 
   @Test
-  public void testEnvVarResolution() throws IOException {
+  public void testEnvVarResolution() throws IOException, ConfigurationException {
     String propertyStr = "var1=foo\n" + "var2=$JAVA_HOME\n" + "var3=${JAVA_HOME}";
 
     Path path = createPropertiesFile(propertyStr);
 
-    Properties props = getResolvedProperties(path);
+    String fileName = jobNameFromPath(path);
+    String dir = path.getParent().toString();
+
+    Properties props = getJobConfigProperties(path.toFile());
 
     Assert.assertNotEquals(props, null);
     Assert.assertEquals(props.getProperty("var1", ""), "foo");
     Assert.assertTrue(props.getProperty("var2").length() > 0);
     Assert.assertEquals(props.getProperty("var2"), props.getProperty("var3"));
-
     Files.deleteIfExists(path);
   }
 
   @Test
-  public void testGetScheduledJobs() throws IOException {
+  public void testGetScheduledJobs() throws IOException, ConfigurationException {
     String propertyStr1 = "job.class=test\n" + "job.cron.expr=0 0 1 * * ? *\n" + "#job.disabled=1\n" + "job.type=TEST1";
     String propertyStr2 = "job.class=test\n" + "job.cron.expr=0 0 1 * * ? *\n" + "job.disabled=1\n" + "job.type=TEST2";
     String propertyStr3 = "job.class=test\n" + "#job.disabled=1\n" + "job.type=TEST3";
@@ -71,7 +74,7 @@ public void testGetScheduledJobs() throws IOException {
   }
 
   @Test
-  public void testGetEnabledJobs() throws IOException {
+  public void testGetEnabledJobs() throws IOException, ConfigurationException {
     String propertyStr1 = "job.class=test\n" + "job.cron.expr=0 0 1 * * ? *\n" + "#job.disabled=1\n" + "job.type=TEST1";
     String propertyStr2 = "job.class=test\n" + "#job.disabled=1\n";
 
@@ -95,9 +98,10 @@ public void testGetEnabledJobs() throws IOException {
   }
 
   @Test
-  public void testGetEnabledJobsByType() throws IOException {
+  public void testGetEnabledJobsByType() throws IOException, ConfigurationException {
     String propertyStr1 = "job.class=test\n" + "job.cron.expr=0 0 1 * * ? *\n" + "#job.disabled=1\n" + "job.type=TEST1";
-    String propertyStr2 = "job.class=test\n" + "job.cron.expr=0 0 1 * * ? *\n" + "#job.disabled=1\n" + "job.type=TEST2";
+    String propertyStr2 =
+        "job.class=test\n" + "job.cron.expr=0 0 1 * * ? *\n" + "#job.disabled=1\n" + "job.type=TEST2,TEST3";
 
     Path path1 = createPropertiesFile(propertyStr1);
     Path path2 = createPropertiesFile(propertyStr2);
@@ -116,6 +120,27 @@ public void testGetEnabledJobsByType() throws IOException {
     Files.deleteIfExists(path2);
   }
 
+  @Test
+  public void testMultipleLinesSameProperties() throws IOException, ConfigurationException {
+    String dir = "/tmp/";
+    String propertyStr =
+        "var1=foo\n" + "var1=foo\n" + "var1=foo\n" + "var2=foo\n" + "var2=foo\n" + "var3=foo\n" + "var1=foo\n"
+            + "var4=foo\n";
+    String propertyStr1 = "var5=foo,bar,test,test1\n" + "var5=test2\n";
+    Properties props = new Properties();
+    Path path = createPropertiesFile(propertyStr + propertyStr1);
+
+    props = getJobConfigProperties(path.toFile());
+
+    Assert.assertNotEquals(props, null);
+    Assert.assertEquals(props.getProperty("var1", ""), "foo,foo,foo,foo");
+    Assert.assertEquals(props.getProperty("var2"), "foo,foo");
+    Assert.assertEquals(props.getProperty("var3"), props.getProperty("var4"));
+    Assert.assertEquals(props.getProperty("var5"), "foo,bar,test,test1,test2");
+
+    Files.deleteIfExists(path);
+  }
+
   private Path createPropertiesFile(String content) throws IOException {
     File propertyFile = File.createTempFile("temp", ".job");
     FileWriter writer = new FileWriter(propertyFile);