Support or null methods

[jroper]

We're slowly switching to using reference.conf for the dual purpose of providing defaults and documenting values. But we have a lot of configuration options where the defaults come from a third party library, for example, a database connection pool provides 20 or so configuration parameters, and it knows best what the defaults should be. So currently, in reference.conf, we do this:

# If defined, sets the foo bar, otherwise defaults to the libraries default value for foo
# foo.bar = 

This has the following problems:

• This documentation is not self tested. Our code might say:

  config.getString("bar.foo")
  

  and that might be the correct way to access it, so the value in reference.conf is wrong, and there's no way for us to test that.

• There's no way to undefine it, so if in development you explicitly set it, but then in your prod.conf, which includes the development application.conf, you want it to select the libraries default, you can't do this.

One possible solution to this is to support or null methods, eg:

config.getStringOrNull("bar.foo")

Then in reference.conf, we can say:

# If not null, sets the foo bar, otherwise defaults to the libraries default value for foo
foo.bar = null

[viktorklang]

If the pool artifact provides the defaults then the pool should have its own reference.conf and you put the docs there?

[jroper]

I just realised - I got this confused with the other issue I raised about prototype objects, so ignore my previous (deleted) comment. The connection pool is third party - BoneCP (soon to move to HikariCP). As much as I'd like them to use Typesafe config, they don't. And there are many similar cases in Play, including async-http-client, jpa, ebean, ehcache, plus the JDK itself, particularly with JSSE, where there are many cases where we want to delegate to the platform default algorithms/ssl providers/etc.

[havocp]

#186 is the existing issue for OrNull methods.
Question: since we require Java 8, would it be better to return Optional ... ?

It may also be worth thinking about ways to merge the default config of these other packages into the reference config, in the same way system properties and environment variables are currently merged in.
Of course, third party APIs may not provide a way to get the defaults, maybe they only allow you to either override them or not, so in that case we'd have a hard time.

One feature we could have is a way to specify code to run as part of loading defaultReference(). Basically a way to register a Config => Config transformer function that we'd run during defaultReference() (with load() in turn calling defaultReference()). This could also be used by people who want to add reference.yaml support or stuff like that.

Another possible idea could be a tag on ConfigOrigin which would indicate whether a value comes from the reference config or the application config. A little bit sketchy to start making that change how the value is interpreted, though.

[jroper]

Merging config is not always possible. For example, often the default is not a configuration item at all, it's an object that has been constructed and initialised by the third party library (eg an SSLEngine), and since Typesafe config doesn't have an SSLEngine data type, that can't be expressed in configuration.

There's also situations where the default is not a static value, but something more dynamic. For example, in Play you can specify a router using play.http.router. If you don't specify one, the framework will default to looking up a class called Routes, but it will only use that if it exists, otherwise, it uses an empty router, play.api.routing.Router.empty. This means the default is Routes if that class exists, or play.api.routing.Router.empty if it doesn't. While we could pre calculate that before loading the configuration, this means that Play must control loading the configuration, it means that users can't embed Play, passing in their own configuration that they've loaded, so it complicates things in that way a bit. It also doesn't feel right looking up the Routes class if it's not going to end up being used, we should only look that up if no router is explicitly configured.

[jroper]

Also something that I was thinking of doing here was writing an sbt plugin that validated Typesafe config configuration to ensure that all the keys that a user has defined were overriding keys in the available reference.conf files. Obviously this would be configurable (only validate certain paths, exclude certain paths, maybe be able to register some transformers for the reference.conf configuration based on the application.conf to ensure it's populated for more dynamic things, this could be used to handle things like the prototype proposal that I made), but it would be really useful for catching errors like typos in configuration, or the use of deprecated keys, etc.

[havocp]

have you seen the existing checkValid method? or suggesting more than that ?

[jroper]

I hadn't seen that, but I'm thinking something more comprehensive. checkValid is only fail fast for things that would otherwise fail anyway. A typo in a configuration item would not fail, it would just be ignored, so the user would be unaware that they have made that typo. So, for example, play provides a configuration item called play.http.session.secure, but what if the user accidentally says play.http.session.ssl = true? This won't fail, since Play never tries to do anything with the ssl property, and in this case, the result is dangerous, their sessions will not have the secure flag set, and unless they check the wire, they won't know.

[viktorklang]

The router situation sound like a list of FQCNs that will be tried in order, would that solve your situation?
Is this a case of designing against the library instead of along it?

[havocp]

aeebeaa shows how this could be done; comments appreciated before I tediously cut-and-paste to every single type, since there are 25-ish types we have getters for.

Now that I did that... a much smaller API addition could be hasPathOrNull ? to avoid an extra 25 methods. Then again a few more methods isn't a huge deal.

[jroper]

hasPathOrNull would need to return a trie state value. I think isNull would suffice - throw an exception if no value was found, return true if null was found, or false if non null was found.

[jroper]

Btw, we've created a type class based API to accessing Typesafe config in Play now, so we can add all sorts of different methods for looking up config (a big one at the moment is looking up deprecated config, falling back to the new config keys, so if the deprecated one is found, it outputs a warning saying that config value is deprecated).

[havocp]

leaning toward just isNull to avoid the tons of methods but will sleep on it and see if anyone else has ideas in the meantime.

[havocp]

Proposing PR #286 to close this.

[havocp]

closed by #286