From 81ce6038c7b465c813400ee8e8dd3eab120db483 Mon Sep 17 00:00:00 2001 From: Havoc Pennington Date: Tue, 29 Nov 2011 15:46:55 -0500 Subject: [PATCH] remove special-case fallback to system props in substitutions. The earlier change to make ${user.home} in a file included underneath "foo" search both ${foo.user.home} and then ${user.home}, means that even in included files system props are picked up just fine as long as they were merged into the root config. So there is no longer any need to have a special-case fallback to system properties. This leaves ConfigResolveOptions as a really complex way to pass in a single bool, but of course the point is to allow for future extension. --- HOCON.md | 40 ++++------------ README.md | 18 +++---- .../com/typesafe/config/ConfigFactory.java | 22 --------- .../typesafe/config/ConfigResolveOptions.java | 48 ++++--------------- .../config/impl/ConfigSubstitution.java | 5 -- .../config/impl/ConfigSubstitutionTest.scala | 7 ++- 6 files changed, 31 insertions(+), 109 deletions(-) diff --git a/HOCON.md b/HOCON.md index 551879f2..ec9526f5 100644 --- a/HOCON.md +++ b/HOCON.md @@ -392,11 +392,6 @@ path expression in a key, because it has a special interpretation Substitutions are a way of referring to other parts of the configuration tree. -For substitutions which are not found in the configuration tree, -implementations may try to resolve them by looking at system -environment variables, Java system properties, or other external -sources of configuration. - The syntax is `${pathexpression}` or `${?pathexpression}` where the `pathexpression` is a path expression as described above. This path expression has the same syntax that you could use for an @@ -406,6 +401,11 @@ The `?` in `${?pathexpression}` must not have whitespace before it; the three characters `${?` must be exactly like that, grouped together. +For substitutions which are not found in the configuration tree, +implementations may try to resolve them by looking at system +environment variables or other external sources of configuration. +(More detail on environment variables in a later section.) + Substitutions are not parsed inside quoted strings. To get a string containing a substitution, you must use value concatenation with the substitution in the unquoted portion: @@ -428,11 +428,6 @@ more than once, the substitution will always evaluate to its latest-assigned value (the merged object or the last non-object value that was set). -If a substitutions does not match any value present in the -configuration, implementations may look up that substitution in -one or more external sources, such as a Java system property or an -environment variable. (More detail on this in a later section.) - If a configuration sets a value to `null` then it should not be looked up in the external source. Unfortunately there is no way to "undo" this in a later configuration file; if you have `{ "HOME" : @@ -962,31 +957,12 @@ For an application's config, Java system properties _override_ settings found in the configuration file. This supports specifying config options on the command line. -### Substitution fallback to system properties +### Substitution fallback to environment variables Recall that if a substitution is not present (not even set to `null`) within a configuration tree, implementations may search -for it from external sources. One such source could be Java system -properties. - -If you merge system properties in to an application's -configuration as overrides, then falling back to them for -substitutions isn't important when parsing a toplevel config file -- they would already be found in the configuration. - -However, there are some cases when system properties fallbacks are -used. For example, when a config file is parsed as an include, its -substitutions are relative to the key the file is included -underneath. In that case, `${user.home}` might become -`${something.user.home}` and would not match a system property -override applied to the root of the config tree. However, -`${user.home}` always falls back to the system property. - -### Substitution fallback to environment variables - -Substitutions not found in the configuration may also fall back to -environment variables. In Java, fallback should be to system -properties first and environment variables second. +for it from external sources. One such source could be environment +variables. It's recommended that HOCON keys always use lowercase, because environment variables generally are capitalized. This avoids diff --git a/README.md b/README.md index 365ff423..9b2d8091 100644 --- a/README.md +++ b/README.md @@ -22,7 +22,7 @@ Configuration library for JVM languages. - substitutions (`"foo" : ${bar}`, `"foo" : Hello ${who}`) - properties-like notation (`a.b=c`) - less noisy, more lenient syntax - - substitute environment variables and system properties + - substitute environment variables This library limits itself to config files. If you want to load config from a database or something, you would need to build a @@ -116,10 +116,10 @@ detail. as the `b` field in the `a` object - substitutions concatenate into unquoted strings, `foo : the quick ${colors.fox} jumped` - - substitutions fall back to system properties and then - environment variables if they don't resolve in the - config itself, so `${HOME}` or `${user.home}` would - work as you expect. + - substitutions fall back to environment variables if they don't + resolve in the config itself, so `${HOME}` would work as you + expect. Also, most configs have system properties merged in so + you could use `${user.home}`. - substitutions normally cause an error if unresolved, but there is a syntax `${?a.b}` to permit them to be missing. @@ -257,10 +257,10 @@ Here are some features that might be nice to add. object merge you have to first set the object to a non-object such as null, then set a new object.) - "delete": allow deleting a field, which is slightly different - from setting it to null (deletion allows fallback to values - in system properties and the environment, for example). - This could be done using the same syntax as `include`, - potentially. It is not a backward-compatible change though. + from setting it to null (deletion allows fallback to values in + the environment, for example). This could be done using the + same syntax as `include`, potentially. It is not a + backward-compatible change though. - substitutions with fallbacks; this could be something like `${foo.bar,baz,null}` where it would look up `foo.bar`, then `baz`, then finally fall back to null. One question is whether diff --git a/config/src/main/java/com/typesafe/config/ConfigFactory.java b/config/src/main/java/com/typesafe/config/ConfigFactory.java index e94112a2..bb6716fb 100644 --- a/config/src/main/java/com/typesafe/config/ConfigFactory.java +++ b/config/src/main/java/com/typesafe/config/ConfigFactory.java @@ -57,17 +57,6 @@ public final class ConfigFactory { /** * Like {@link #load(String)} but allows you to specify parse and resolve * options. - *

- * To be aware of: using - * {@link ConfigResolveOptions#setUseSystemProperties(boolean) - * setUseSystemProperties(false)} with this method has little effect, - * because the system properties are merged into the config as overrides - * anyway. setUseSystemProperties affects whether to fall back - * to system properties when they are not found in the config, but with - * load(), they will be in the config. There is one situation - * where setUseSystemProperties(false) comes into play, which - * is that files included into another file may find the system property - * fallbacks but not the overrides. * @param resourceBasename * the classpath resource name with optional extension * @param parseOptions @@ -100,17 +89,6 @@ public final class ConfigFactory { /** * Like {@link #load(Config)} but allows you to specify * {@link ConfigResolveOptions}. - *

- * To be aware of: using - * {@link ConfigResolveOptions#setUseSystemProperties(boolean) - * setUseSystemProperties(false)} with this method has little effect, - * because the system properties are merged into the config as overrides - * anyway. setUseSystemProperties affects whether to fall back - * to system properties when they are not found in the config, but with - * load(), they will be in the config. There is one situation - * where setUseSystemProperties(false) comes into play, which - * is that files included into another file may find the system property - * fallbacks but not the overrides. * * @param config * the application's portion of the configuration diff --git a/config/src/main/java/com/typesafe/config/ConfigResolveOptions.java b/config/src/main/java/com/typesafe/config/ConfigResolveOptions.java index 8ed11c4b..37c7b36d 100644 --- a/config/src/main/java/com/typesafe/config/ConfigResolveOptions.java +++ b/config/src/main/java/com/typesafe/config/ConfigResolveOptions.java @@ -7,31 +7,24 @@ package com.typesafe.config; * A set of options related to resolving substitutions. Substitutions use the * ${foo.bar} syntax and are documented in the HOCON spec. - * *

* This object is immutable, so the "setters" return a new object. - * *

* Here is an example of creating a custom {@code ConfigResolveOptions}: - * * 

  *     ConfigResolveOptions options = ConfigResolveOptions.defaults()
- *         .setUseSystemProperties(false)
  *         .setUseSystemEnvironment(false)
  *
- * *

* In addition to {@link ConfigResolveOptions#defaults}, there's a prebuilt * {@link ConfigResolveOptions#noSystem} which avoids looking at any system - * properties or environment variables. + * environment variables or other external system information. (Right now, + * environment variables are the only example.) */ public final class ConfigResolveOptions { - private final boolean useSystemProperties; private final boolean useSystemEnvironment; - private ConfigResolveOptions(boolean useSystemProperties, - boolean useSystemEnvironment) { - this.useSystemProperties = useSystemProperties; + private ConfigResolveOptions(boolean useSystemEnvironment) { this.useSystemEnvironment = useSystemEnvironment; } @@ -41,31 +34,17 @@ public final class ConfigResolveOptions { * @return the default resolve options */ public static ConfigResolveOptions defaults() { - return new ConfigResolveOptions(true, true); + return new ConfigResolveOptions(true); } /** * Returns resolve options that disable any reference to "system" data - * (system properties or environment variables). + * (currently, this means environment variables). * - * @return the resolve options with system properties and env variables - * disabled + * @return the resolve options with env variables disabled */ public static ConfigResolveOptions noSystem() { - return defaults().setUseSystemEnvironment(false).setUseSystemProperties(false); - } - - /** - * Returns options with use of Java system properties set to the given - * value. - * - * @param value - * true to resolve substitutions falling back to Java system - * properties. - * @return options with requested setting for use of system properties - */ - public ConfigResolveOptions setUseSystemProperties(boolean value) { - return new ConfigResolveOptions(value, useSystemEnvironment); + return defaults().setUseSystemEnvironment(false); } /** @@ -76,18 +55,9 @@ public final class ConfigResolveOptions { * variables. * @return options with requested setting for use of environment variables */ + @SuppressWarnings("static-method") public ConfigResolveOptions setUseSystemEnvironment(boolean value) { - return new ConfigResolveOptions(useSystemProperties, value); - } - - /** - * Returns whether the options enable use of system properties. This method - * is mostly used by the config lib internally, not by applications. - * - * @return true if system properties should be used - */ - public boolean getUseSystemProperties() { - return useSystemProperties; + return new ConfigResolveOptions(value); } /** diff --git a/config/src/main/java/com/typesafe/config/impl/ConfigSubstitution.java b/config/src/main/java/com/typesafe/config/impl/ConfigSubstitution.java index 390ec893..8f1b4357 100644 --- a/config/src/main/java/com/typesafe/config/impl/ConfigSubstitution.java +++ b/config/src/main/java/com/typesafe/config/impl/ConfigSubstitution.java @@ -146,11 +146,6 @@ final class ConfigSubstitution extends AbstractConfigValue implements result = findInObject(resolver.root(), resolver, unprefixed, depth, options); } - if (result == null && options.getUseSystemProperties()) { - result = findInObject(ConfigImpl.systemPropertiesAsConfigObject(), null, - unprefixed, depth, options); - } - if (result == null && options.getUseSystemEnvironment()) { result = findInObject(ConfigImpl.envVariablesAsConfigObject(), null, unprefixed, depth, options); diff --git a/config/src/test/scala/com/typesafe/config/impl/ConfigSubstitutionTest.scala b/config/src/test/scala/com/typesafe/config/impl/ConfigSubstitutionTest.scala index f65670cc..73da3444 100644 --- a/config/src/test/scala/com/typesafe/config/impl/ConfigSubstitutionTest.scala +++ b/config/src/test/scala/com/typesafe/config/impl/ConfigSubstitutionTest.scala @@ -322,13 +322,16 @@ class ConfigSubstitutionTest extends TestUtils { """) } + // 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. @Test - def fallbackToSystemProps() { + def overrideWithSystemProps() { System.setProperty("configtest.a", "1234") System.setProperty("configtest.b", "5678") ConfigImpl.reloadSystemPropertiesConfig() - val resolved = resolve(substSystemPropsObject) + val resolved = resolve(ConfigFactory.systemProperties().withFallback(substSystemPropsObject).root.asInstanceOf[AbstractConfigObject]) assertEquals("1234", resolved.getString("a")) assertEquals("5678", resolved.getString("b"))