Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow environment vars to be resolved to list #427

Merged
merged 2 commits into from
Mar 6, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions config/build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
11 changes: 1 addition & 10 deletions config/src/main/java/com/typesafe/config/impl/ConfigImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down Expand Up @@ -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.
Expand Down
16 changes: 12 additions & 4 deletions config/src/test/scala/com/typesafe/config/impl/PublicApiTest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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).
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good find! I imagine that was some work to track down...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it took few hours. I had to use windows OS because on linux I wasn't able to reproduce the issue - it was horrible :)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ouch, I know the feeling :-)

TimeZone.getDefault
}

@Test
def basicLoadAndGet() {
val conf = ConfigFactory.load("test01")
Expand Down Expand Up @@ -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"))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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)
Expand Down