Merge pull request lightbend#427 from psliwa/patch-1

Allow environment vars to be resolved to list

config/build.sbt

@@ -14,9 +14,13 @@ ScalariformKeys.preferences in Compile := formatPrefs
 ScalariformKeys.preferences in Test := formatPrefs
 
 fork in test := true
+fork in Test := true
 fork in run := true
 fork in run in Test := true
 
+//env vars for tests
+envVars in Test ++= Map("testList.0" -> "0", "testList.1" -> "1")
+
 autoScalaLibrary := false
 crossPaths := false
 

config/src/main/java/com/typesafe/config/impl/ConfigImpl.java

@@ -335,16 +335,7 @@ public static void reloadSystemPropertiesConfig() {
     }
 
     private static AbstractConfigObject loadEnvVariables() {
-        Map<String, String> env = System.getenv();
-        Map<String, AbstractConfigValue> m = new HashMap<String, AbstractConfigValue>();
-        for (Map.Entry<String, String> entry : env.entrySet()) {
-            String key = entry.getKey();
-            m.put(key,
-                    new ConfigString.Quoted(SimpleConfigOrigin.newSimple("env var " + key), entry
-                            .getValue()));
-        }
-        return new SimpleConfigObject(SimpleConfigOrigin.newSimple("env variables"),
-                m, ResolveStatus.RESOLVED, false /* ignoresFallbacks */);
+        return PropertiesParser.fromStringMap(newSimpleOrigin("env variables"), System.getenv());
     }
 
     private static class EnvVariablesHolder {

config/src/main/java/com/typesafe/config/impl/PropertiesParser.java

@@ -56,15 +56,28 @@ static Path pathFromPropertyKey(String key) {
 
     static AbstractConfigObject fromProperties(ConfigOrigin origin,
             Properties props) {
+        return fromEntrySet(origin, props.entrySet());
+    }
+
+    private static <K, V> AbstractConfigObject fromEntrySet(ConfigOrigin origin, Set<Map.Entry<K, V>> entries) {
+        final Map<Path, Object> pathMap = getPathMap(entries);
+        return fromPathMap(origin, pathMap, true /* from properties */);
+    }
+
+    private static <K, V> Map<Path, Object> getPathMap(Set<Map.Entry<K, V>> entries) {
         Map<Path, Object> pathMap = new HashMap<Path, Object>();
-        for (Map.Entry<Object, Object> entry : props.entrySet()) {
+        for (Map.Entry<K, V> entry : entries) {
             Object key = entry.getKey();
             if (key instanceof String) {
                 Path path = pathFromPropertyKey((String) key);
                 pathMap.put(path, entry.getValue());
             }
         }
-        return fromPathMap(origin, pathMap, true /* from properties */);
+        return pathMap;
+    }
+
+    static AbstractConfigObject fromStringMap(ConfigOrigin origin, Map<String, String> stringMap) {
+        return fromEntrySet(origin, stringMap.entrySet());
     }
 
     static AbstractConfigObject fromPathMap(ConfigOrigin origin,

config/src/test/scala/com/typesafe/config/impl/ConfigDocumentTest.scala

@@ -292,7 +292,7 @@ class ConfigDocumentTest extends TestUtils {
     @Test
     def configDocumentFileParse {
         val configDocument = ConfigDocumentFactory.parseFile(resourceFile("/test03.conf"))
-        val fileReader = new BufferedReader(new FileReader("config/src/test/resources/test03.conf"))
+        val fileReader = new BufferedReader(new FileReader("src/test/resources/test03.conf"))
         var line = fileReader.readLine()
         val sb = new StringBuilder()
         while (line != null) {

config/src/test/scala/com/typesafe/config/impl/ConfigSubstitutionTest.scala

@@ -10,6 +10,7 @@ import com.typesafe.config.ConfigException
 import com.typesafe.config.ConfigResolveOptions
 import com.typesafe.config.Config
 import com.typesafe.config.ConfigFactory
+import scala.collection.JavaConverters._
 
 class ConfigSubstitutionTest extends TestUtils {
 
@@ -723,6 +724,35 @@ class ConfigSubstitutionTest extends TestUtils {
         checkNotSerializable(substComplexObject)
     }
 
+    @Test
+    def resolveListFromSystemProps() {
+        val props = parseObject(
+            """
+            |"a": ${testList}
+            """.stripMargin)
+
+        System.setProperty("testList.0", "0")
+        System.setProperty("testList.1", "1")
+        ConfigImpl.reloadSystemPropertiesConfig()
+
+        val resolved = resolve(ConfigFactory.systemProperties().withFallback(props).root.asInstanceOf[AbstractConfigObject])
+
+        assertEquals(List("0", "1"), resolved.getList("a").unwrapped().asScala)
+    }
+
+    @Test
+    def resolveListFromEnvVars() {
+        val props = parseObject(
+            """
+            |"a": ${testList}
+            """.stripMargin)
+
+        //"testList.0" and "testList.1" are defined as envVars in build.sbt
+        val resolved = resolve(props)
+
+        assertEquals(List("0", "1"), resolved.getList("a").unwrapped().asScala)
+    }
+
     // this is a weird test, it used to test fallback to system props which made more sense.
     // Now it just tests that if you override with system props, you can use system props
     // in substitutions.

config/src/test/scala/com/typesafe/config/impl/PublicApiTest.scala

@@ -7,8 +7,7 @@ import org.junit.Assert._
 import org.junit._
 import scala.collection.JavaConverters._
 import com.typesafe.config._
-import java.util.Collections
-import java.util.TreeSet
+import java.util.{ Collections, TimeZone, TreeSet }
 import java.io.File
 import scala.collection.mutable
 import equiv03.SomethingInEquiv03
@@ -17,6 +16,15 @@ import java.net.URL
 import java.time.Duration
 
 class PublicApiTest extends TestUtils {
+
+    @Before
+    def before(): Unit = {
+        // TimeZone.getDefault internally invokes System.setProperty("user.timezone", <default time zone>) and it may
+        // cause flaky tests depending on tests order and jvm options. This method is invoked
+        // eg. by URLConnection.getContentType (it reads headers and gets default time zone).
+        TimeZone.getDefault
+    }
+
     @Test
     def basicLoadAndGet() {
         val conf = ConfigFactory.load("test01")
@@ -1016,8 +1024,8 @@ class PublicApiTest extends TestUtils {
         assertTrue("invalidate caches works on changed system props sys", sys2 ne sys3)
         assertTrue("invalidate caches works on changed system props conf", conf2 ne conf3)
 
-        assertTrue("invalidate caches doesn't change value if no system prop changes sys", sys1 == sys2)
-        assertTrue("invalidate caches doesn't change value if no system prop changes conf", conf1 == conf2)
+        assertEquals("invalidate caches doesn't change value if no system prop changes sys", sys1, sys2)
+        assertEquals("invalidate caches doesn't change value if no system prop changes conf", conf1, conf2)
 
         assertTrue("test system property is set sys", sys3.hasPath("invalidateCachesTest"))
         assertTrue("test system property is set conf", conf3.hasPath("invalidateCachesTest"))

config/src/test/scala/com/typesafe/config/impl/TestUtils.scala

@@ -721,7 +721,7 @@ abstract trait TestUtils {
     def path(elements: String*) = new Path(elements: _*)
 
     val resourceDir = {
-        val f = new File("config/src/test/resources")
+        val f = new File("src/test/resources")
         if (!f.exists()) {
             val here = new File(".").getAbsolutePath
             throw new Exception(s"Tests must be run from the root project directory containing ${f.getPath()}, however the current directory is $here")
@@ -871,7 +871,7 @@ abstract trait TestUtils {
     }
 
     protected def withScratchDirectory[T](testcase: String)(body: File => T): Unit = {
-        val target = new File("config/target")
+        val target = new File("target")
         if (!target.isDirectory)
             throw new RuntimeException(s"Expecting $target to exist")
         val suffix = java.lang.Integer.toHexString(java.util.concurrent.ThreadLocalRandom.current.nextInt)