config icon indicating copy to clipboard operation
config copied to clipboard

No way to declare key/values (containing dots) sharing a part of the "path"

Open lucabelluccini opened this issue 7 years ago • 25 comments

Hello, we're using this great library to store our process configuration. We are also passing some property to underlying 3rd party library.

Is there a way to have the following properties in the config file?

xxx.yyy=true
xxx.yyy.zzz="Something"

Internally, the library will just generate:

xxx { yyy { zzz : Something }}

But what I would like to archive is:

xxx.yyy : True
xxx.yyy.zzz : "Something"

Using quotes allows to keep the keys as distinct values, but the keys will include the quotes in the value of the key.

lucabelluccini avatar Sep 06 '17 19:09 lucabelluccini

{ "xxx.yyy"=true "xxx.yyy.zzz"="Something" }

go with this

He-Pin avatar Sep 06 '17 19:09 He-Pin

Hello, as stated in the issue content, the following:

{
  "es.net.ssl.cert.allow.self.signed"=true
  "es.net.ssl"=true
}

Requires you to write the following code:

    val appConf = ConfigFactory.load()
    log.info(appConf.getString("\"es.net.ssl\"")) // true
    log.info(appConf.getString("\"es.net.ssl.cert.allow.self.signed\"")) // true

The quote is part of the key.

lucabelluccini avatar Sep 07 '17 07:09 lucabelluccini

yyy can't be both a boolean value and also an object that contains another boolean value. a.b=true is a shorthand for a:{b:true}

"a.b"=true is shorthand for {"a.b":true} (see the discussion of keys vs paths in the docs)

havocp avatar Sep 07 '17 11:09 havocp

I read the doc and I understand the decision. But I find HOCON limits the possibility to have keys with dots. In json, it would be completely ok to do the following:

{
"xxx.yyy": true,
"xxx.yyy.zzz": false
}

With the library, it's not possible except escaping the quotes "manually" in the code.

lucabelluccini avatar Sep 07 '17 14:09 lucabelluccini

HOCON is a json superset so you can still write that syntax in the file if you want.

Most of the API takes path expressions,a.b means two keys unless you quote it to mean one key. There is API on ConfigObject though that takes keys instead of paths; conf.toObject.get would not require quotes.

If the getters didn't take path expressions you would have to do something like conf.getConfig("a").getConfig("b").getBoolean("c") which is quite clunky so that's why Config uses paths.

There is a utility method (in ConfigUtil maybe?) to quote keys for you if desired.

havocp avatar Sep 07 '17 15:09 havocp

Hello, trying to use this great library, but we have to support property configs with settings like

xxx.yyy=true
xxx.yyy.zzz="Something"

Is it possible to make properties parsing compatible to such settings style? Value from key "xxx.yyy" could be paced inside "xxx.yyy" object with any reserved key.

Here is simple change in function com.typesafe.config.impl.PropertiesParser.fromPathMap(ConfigOrigin, Map<Path, Object>, boolean) implementing this logic:

....
        if (convertedFromProperties) {
            /*
             * If any string values are also objects containing other values,
             * push those string values into the objects with key "__value__".
             */
            //valuePaths.removeAll(scopePaths);
            for (Path path : scopePaths) {
                if (valuePaths.remove(path)) {
                    Path primitivePath = Path.newKey("__value__").prepend(path);
                    if (valuePaths.contains(primitivePath) || scopePaths.contains(primitivePath))
                        // reserved subkey should not rewrite another subkey
                        continue;
                    valuePaths.add(primitivePath);
                    pathMap.put(primitivePath, pathMap.remove(path));
                }
            }
        } else {
....

(actually, we use modified library version due to java1.7 stack and have applied this change. Looks like it works fine)

radist-nt avatar Jul 07 '18 22:07 radist-nt

Might be mixing some different things into this issue (representing dots in keys in the Config data structure, vs. representing values-stored-at-parent-nodes).

The __value__ idea would be a somewhat leaky abstraction... e.g. rendering the config would show that as "__value__" : true, and you'd have to know the magic value name to get that value... I have mixed feelings about it. Wish we could come up with something more elegant.

havocp avatar Jul 11 '18 15:07 havocp

I've realized that putting property value as value object key in the case of conflict was not a good idea. We have dynamic config file like:

base.object1 = class.name.First
base.object2 = class.name.Second
base.object2.someconfig = some second config
base.anotherobject = class.name.Second
base.anotherobject.someconfig = another second config

And each __value__ insertion should be handled with several lines of code like:

            // configPath contains the path to dynamic config value or object
            String value;
            Config subValues;
            switch (myConfig.root().get(configPath).valueType()) {
            case STRING:
                // no sub-values
                value = myConfig.getString(configPath);
                subValues = ConfigFactory.empty();
                break;
            case OBJECT:
                Config cnf = myConfig.getConfig(configPath);
                value = cnf.getString("__value__");
                subValues = cnf.withoutPath("__value__");
                break;
            default:
                throw new ConfigException.BadValue(configPath + " parameter has invalid type");
            }

The only solution is to save the conflicting value inside the ConfigObject and read this value by Config.getXxx instead of throwing exception (if value is set). It should not affect the performance when using JSON/HOCON or properties without conflicts in the case of valid config.

radist-nt avatar Jul 14 '18 23:07 radist-nt

As someone who's dealing with migrating from an old properties file (that we can't deprecate right away), I've often wished the lib had support for something like this as it seems to be a somewhat-common practice in .properties files.

bbaldino avatar Jan 13 '20 23:01 bbaldino

We managed to successfully migrate from properties file. We've adopted the following three rules:

  • xxx = true/false; xxx.yyy = ...: typically xxx.yyy doesn't make sence for xxx = false, so we dropping xxx in favour of main.hasPath("xxx")
  • other conflicts typically solved by inserting conflicting non-object value into conflicting object
  • lists denoted as string-with-separators should be converted to the HOCON lists or to the HOCON object (for string lists).

It's very usefull to write one simple program for config migration which reads the properties, changes the key names for conflicting properties, checks the absence of conflicts and converts is to HOCON. The core code looks like this:

private static Map<String, String> KEY_MAPPINGS = Stream
            .of(new SimpleImmutableEntry<>("conflicting.prop1", "old.conflicted.prop1.newpath"),
                    new SimpleImmutableEntry<>("another.conflicting.prop2", "another.conflicting.prop2.newpath"), ...)
            .collect(Collectors.toMap(Entry::getKey, Entry::getValue));
private static final Map<String, String> DURATION_PROPS = Stream
            .of(new SimpleImmutableEntry<>("some.duration.prop1", "s"),
                    new SimpleImmutableEntry<>("another.duration.prop2", "m"),...)
            .collect(Collectors.toMap(Entry::getKey, Entry::getValue));
private static final Pattern NOT_DOT = Pattern.compile("[^.]+");
/** Solves and checks conflicts BEFORE converting Properties to Config. Does not covers converting char-separated-lists to HOCON lists (it should be done in Config after the convertation). */
public static boolean transformProperties(Properties props) {
    // nest conflicting keys into inner prop
    KEY_MAPPINGS.keySet().stream().filter(props::containsKey).forEach(e -> props.put(e.getValue(), props.remove(e.getKey())));
    // append duration dimensions
    DURATION_PROPS.keySet().stream().filter(props::containsKey)
            .forEach(k -> props.setProperty(k, props.getProperty(k) + DURATION_PROPS.get(k)));
    // put any other convertation code here (specific to your app)

    // test for conflicts
    List<String> allProps = new ArrayList<>(props.stringPropertyNames());
    Collections.sort(allProps, new Comparator<String>() {
        @Override
        public int compare(String s1, String s2) {
            return -Integer.compare(NOT_DOT.matcher(s1).replaceAll("").length(),
                    NOT_DOT.matcher(s2).replaceAll("").length());
        }
    });
    Set<String> foundProps = new HashSet<>();
    boolean foundConflicts  = false;
    for (String prop : allProps) {
        if (foundProps.contains(prop)) {
            System.err.println("property conflict: " + prop);
            foundConflicts = true;
        }
        int lastIndexOf = prop.lastIndexOf('.');
        if (lastIndexOf == -1) {
            lastIndexOf = prop.length();
        }
        foundProps.add(prop.substring(0, lastIndexOf));
    }
    return !foundConflicts;
}

radist-nt avatar May 27 '20 19:05 radist-nt

It would be great to have quoted periods in names like

xxx.yyy: 1
xxx.yyy\.zzz: 2

For example, if I want to add a domain definitions in the configuration:

domains {
  google\.com {
  }

  www\.google\.com {
  }

  github\.com {
  }
}

and retrieve all keys (domain names) there is no convenient way to do it in the current format. Of course, there is a workaround, but it is not very good...

eshu avatar Nov 30 '20 16:11 eshu

I would quote the string like "google.com" ... doesn't seem like a workaround to me?

havocp avatar Nov 30 '20 16:11 havocp

What will return cfg.getConfig("domains").root().keySet() in this case?

eshu avatar Nov 30 '20 17:11 eshu

root() gets you a ConfigObject so the key set will be the JSON-style string keys, not path expressions. Since "google.com" was quoted it will be one key so the keySet should contain the full domain names Set("google.com", "www.google.com", "github.com")

havocp avatar Nov 30 '20 17:11 havocp

Yes, I see now... But then I met another pitfall:

@ val cfg = ConfigFactory.parseString("""domains {"google.com" {}, "www.google.com" {}, "github.com" {}}""") 
cfg: Config = Config(SimpleConfigObject({"domains":{"github.com":{},"google.com":{},"www.google.com":{}}}))

@ val domains = cfg.getConfig("domains") 
domains: Config = Config(SimpleConfigObject({"github.com":{},"google.com":{},"www.google.com":{}}))

@ domains.root.keySet 
res13: java.util.Set[String] = [google.com, github.com, www.google.com]

@ domains.root.keySet.asScala map (domains.getConfig) 
com.typesafe.config.ConfigException$Missing: String: 1: No configuration setting found for key 'google'
  com.typesafe.config.impl.SimpleConfig.findKeyOrNull(SimpleConfig.java:157)
  com.typesafe.config.impl.SimpleConfig.findKey(SimpleConfig.java:150)
  com.typesafe.config.impl.SimpleConfig.findOrNull(SimpleConfig.java:177)
  com.typesafe.config.impl.SimpleConfig.find(SimpleConfig.java:189)
  com.typesafe.config.impl.SimpleConfig.find(SimpleConfig.java:194)
  com.typesafe.config.impl.SimpleConfig.getObject(SimpleConfig.java:269)
  com.typesafe.config.impl.SimpleConfig.getConfig(SimpleConfig.java:275)
  com.typesafe.config.impl.SimpleConfig.getConfig(SimpleConfig.java:42)
  ammonite.$sess.cmd15$.$anonfun$res15$1(cmd15.sc:1)
  scala.collection.Iterator$$anon$9.next(Iterator.scala:573)
  scala.collection.mutable.Growable.addAll(Growable.scala:62)
  scala.collection.mutable.Growable.addAll$(Growable.scala:59)
  scala.collection.mutable.HashSet.addAll(HashSet.scala:100)
  scala.collection.mutable.HashSet$.from(HashSet.scala:30)
  scala.collection.mutable.HashSet$.from(HashSet.scala:371)
  scala.collection.IterableOps.map(Iterable.scala:672)
  scala.collection.IterableOps.map$(Iterable.scala:672)
  scala.collection.AbstractIterable.map(Iterable.scala:921)
  ammonite.$sess.cmd15$.<clinit>(cmd15.sc:1)


@ domains.getConfig("google.com") 
com.typesafe.config.ConfigException$Missing: String: 1: No configuration setting found for key 'google'
  com.typesafe.config.impl.SimpleConfig.findKeyOrNull(SimpleConfig.java:157)
  com.typesafe.config.impl.SimpleConfig.findKey(SimpleConfig.java:150)
  com.typesafe.config.impl.SimpleConfig.findOrNull(SimpleConfig.java:177)
  com.typesafe.config.impl.SimpleConfig.find(SimpleConfig.java:189)
  com.typesafe.config.impl.SimpleConfig.find(SimpleConfig.java:194)
  com.typesafe.config.impl.SimpleConfig.getObject(SimpleConfig.java:269)
  com.typesafe.config.impl.SimpleConfig.getConfig(SimpleConfig.java:275)
  com.typesafe.config.impl.SimpleConfig.getConfig(SimpleConfig.java:42)
  ammonite.$sess.cmd16$.<clinit>(cmd16.sc:1)


@ domains.getConfig("\"google.com\"") 
res17: Config = Config(SimpleConfigObject({}))

@ domains.root.get("google.com") 
res18: ConfigValue = SimpleConfigObject({})

@ domains.root.get("google") 
res19: ConfigValue = null

res18 and res19 looks really fine, but exceptions and res17 looks inconsistent.

Anyway, I use ConfigObject only in my wrapper, it is not a problem for me, but probably you agree that such behavior of Config looks a bit strange. In other hand I already understand complexity of the situation...

eshu avatar Dec 01 '20 01:12 eshu

Config is keyed by path expressions; ConfigObject is keyed by unparsed strings. Conceptually it is a type error to pass a path expression where a string is expected or vice versa. But the Java type String is used for both so the compiler can't flag the error.

domains.root.keySet.asScala map (domains.getConfig) for example shouldn't compile, if we were using distinct types instead of String for everything. It does compile since both types are represented by String but it is still an incorrect line of code that passes the wrong type of String to getConfig.

The res17 example is parsing the string as an expression, res18 and res19 are not.

ConfigObject represents a JSON-style object value, Config represents a configuration.

havocp avatar Dec 01 '20 01:12 havocp

Thank you for explanation and for HOCON itself :)

eshu avatar Dec 01 '20 01:12 eshu

Hey, I have a question related to this issue. Given we have either of these:

xxx.yyy.zzz="Something"
xxx {
  yyy { 
      zzz="Something"
  }
}

We can override it with this env variable: CONFIG_FORCE_xxx_yyy_zzz

But if we have this:

xxx {
  "yyy.zzz"=Something
}

The CONFIG_FORCE_xxx_yyy_zzz can't override it. How can I change it from env?

mdaliyan avatar Oct 20 '21 02:10 mdaliyan

@mdaliyan Use an optional system or override variable

xxx {
  "yyy.zzz": Something
  "yyy.zzz": ${?YOUR_SYSTEM_ENV_HERE}
}

austinxhoffmann avatar Aug 15 '22 18:08 austinxhoffmann

@mdaliyan Use an optional system or override variable

xxx {
  "yyy.zzz": Something
  "yyy.zzz": ${?YOUR_SYSTEM_ENV_HERE}
}

It looks like it will work. Thank you. I'll test it later, I don't wanna touch the code that is working now 😄

mdaliyan avatar Oct 27 '22 01:10 mdaliyan

@mdaliyan Figured you had a working solution a year later, just wanted to post in case anyone stumbled across this from google with the same question

austinxhoffmann avatar Oct 27 '22 20:10 austinxhoffmann

I was actually a little bit distracted and I thought this is the answer I wanted. Though the issue was something else.

Imagine you already have a config in a yaml file, but you want to override it with env variable. By doing this:

xxx {
  "yyy.zzz": Something
  "yyy.zzz": ${?YOUR_SYSTEM_ENV_HERE}
}

we say the app needs to get the value only from env variable which is not what we want. Probably this is the way we want it work if the library supports it. This way if we don't set the env variable we are good to go.

xxx {
  "yyy.zzz": Something
  "yyy.zzz": ${?YOUR_SYSTEM_ENV_HERE:default_value}
}

mdaliyan avatar Nov 18 '22 03:11 mdaliyan

You don't need ":default_value" here, because the default is Something in this example.

radist-nt avatar Nov 18 '22 08:11 radist-nt

Wait a moment, is this even valid to have the same key twice?

mdaliyan avatar Nov 28 '22 14:11 mdaliyan

@mdaliyan , the following documentation sections contain complete answer to your question: https://github.com/lightbend/config/blob/main/HOCON.md#duplicate-keys-and-object-merging https://github.com/lightbend/config/blob/main/HOCON.md#substitutions

radist-nt avatar Nov 29 '22 08:11 radist-nt