Using of EnvironmentConfigLoader based provider requires all the intermediate 'nodes' to be declared

[Mingela]

Hello! Firstly I used an OverrideConfigProvider combining two proxy config providers based on both an environment config loader and a property config loader (from the properties file) and everything worked well. Then I decided to refactor the code to be able to load configs from the environment with an opportunity to use a secondary properties source if the config is not consistent after the 'first' load. I began with validating the loaded config structure but failed because I thought only about the reflection API possibilities. (Would appreciate a piece of advice if a one can perform the consistency validation somehow). So I managed to change the solution to a worse one - I just return 'environment provider' instead of mixed one in case of a file reading failure.

And here comes the main question. So if I try to load a config using ONLY environment variables there must be contained each 'intermediate node'. So if I have the following picture in my environment ('properties' is a prefix):

PROPERTIES_GROUPONE_KEYONE -> "VALUEONE"

I just get an Exception: com.jdiazcano.cfg4k.utils.SettingNotFound: Setting 'properties.groupone' not found. on an attempt to get the value. But if I add:

PROPERTIES_GROUPONE -> ""
PROPERTIES_GROUPONE_KEYONE -> "VALUEONE"
PROPERTIES_GROUPONE_KEYTWO -> "VALUETWO"

everything gets working. Is this behavior incorrect? I do not need to include in my properties file each 'intermediate node' though. But I do in case of working with the environment.

Thank you!

[jdiazcano]

This is definitely a bug, there should be no need to have the root one. I'll try to handle this tonight!

[jdiazcano]

Editing as I've been able to reproduce it. The problem is in the binding not in the getting of the properties!

class EnvironmentConfigLoaderTest : Spek({

    describe("a property config loader") {
        mockkStatic(System::class)
        every { System.getenv() } returns mapOf(
                "PROPERTIES_GROUPONE_KEYONE" to "1",
                "PROPERTIES_GROUPONE_KEYTWO" to "2"
        )

        val loader = EnvironmentConfigLoader()
        it("it should be good in the loader") {
            loader.get("properties.groupone.keyone").should.be.equal("1".toConfig())
        }

        val provider = OverrideConfigProvider(ProxyConfigProvider(loader))

        it ("should be also good with the provider") {
            provider.get<String>("properties.groupone.keyone").should.be.equal("1")
        }

        it("works with binding") {
            val properties = provider.bind<Props>("properties")
            properties.groupone.keyone.should.be.equal("1")
            properties.groupone.keytwo.should.be.equal("2")
        }

        unmockkAll()
    }

})

interface Props {
    val groupone: Groupone
}

interface Groupone {
    val keyone: String
    val keytwo: String
}

[jdiazcano]

TLDR: I have seen what is wrong but sadly I've found a bug with the new gradle/ohmyzsh/WSL that crashes my computer so it might be done tomorrow not today.

I have the fix which is basically using the same engine than the PropertiesConfigLoader is using.

---

Long explanation is that the environment properties worked the other way as everything else, always looking for System.getenv of the transformations of whatever you wanted to get. The problem comes when it tries to get the config object of something that hasn't been created which is the exception it was throwing.

By changing how it is done, now it can use the same engine than the PropertiesConfigLoader is using hence everything works properly.

Changes:

1. Now there is no method addTransformer I've chosen to remove it due to the complexity and even performance it would incur if this happens everytime there is an additional transformer. If someone is actually relying on that, it is possible to add it back but for now I prefer to keep it simple.
2. Instead of the addTransformer now the transformers are passed with the constructor, so in reality there should be no loss as it is still configurable. This actually led me to be able to apply the transformations BEFORE actually saving the root ConfigObject. Before, it would work the other way around, everytime get() was called, the transformers would try to get the environment property and now everything is normalized to the actual ConfigObject, reloading will refresh the root object and we are all good.

Just tried pushing in a branch so CI will work for me: https://travis-ci.org/jdiazcano/cfg4k/builds/515442978

[jdiazcano]

Hopefully should be fixed! For now not released but can be used with jitpack

repositories {
    maven { url 'https://jitpack.io' }
}

2. Add the dependency: com.github.jdiazcano:cfg4k:-MASTER

[Mingela]

Hopefully should be fixed! For now not released but can be used with jitpack

repositories {
    maven { url 'https://jitpack.io' }
}

1. Add the dependency: com.github.jdiazcano:cfg4k:-MASTER

Thank you very much Javier for the quick exhaustive reply and fix! Tried on com.github.jdiazcano:cfg4k:master-SNAPSHOT, worked well! Looking forward to see the fix in a release 😃

[jdiazcano]

There's still something else I want to do, as well as cleaning up a little bit the code and remaking the whole test suite to get rid of spek 1. So it might take some time before the next release, if it is important for you to get it released, please ping me.

[Alexey-N-Chernyshov]

Hi, @jdiazcano! Have you had time to look into this?

[jdiazcano]

The release is still not done (I encountered a Kotlin plugin bug, so I stopped for a while so they can have a way of reproducing it) but this is usable with jitpack. I'm also busy with my current work so it is hard for me to get a lot of time :(

The tests are 90% remade so it'll be done maybe this weekend.

[jdiazcano]

Just published the release 0.9.4!

[Mingela]

Just published the release 0.9.4!

Thank you @jdiazcano! But it seems the fix is not contained there 😨 My test works with the 57-SNAPSHOT version but fails on both 0.9.4 and master-SNAPSHOT (worked earlier) (same com.jdiazcano.cfg4k.utils.SettingNotFound: Setting 'properties.groupone' not found.)

[jdiazcano]

I did a bad merge, could you please try with master-SNAPSHOT before I release 0.9.4.1?

[Mingela]

Sure! It works on both db1460b781 and -SNAPSHOT versions but fails on master-SNAPSHOT. Most probably it is about some jitpack issue. These three versions must have got the same contents.

So I think everything should be fine now.

[jdiazcano]

I see, I'll release a new one today then, anyways if you can confirm the test in the environmentconfigloadertest would be even better

[jdiazcano]

Ok, 0.9.41 release is there now!