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

Substitutions are broken when using Config.resolveWith in a merged node that is a direct decendant of the root node #664

Open
liljencrantz opened this issue Jan 23, 2020 · 3 comments

Comments

@liljencrantz
Copy link

Trivial reproduction:

        val fallback = parseConfig("a: 123")
        val conf = parseConfig("b: 234, a: ${b}")
        val merged = conf
          .withFallback(fallback)
          .resolveWith(
              ConfigFactory.empty(),
              ConfigResolveOptions.defaults().setAllowUnresolved(true)
          ).resolve()

This is somewhat similar to various other issues in the issue tracker.

There is a trivial fix, namely to return this on line 245 of ResolveSource.java. That code even exists but is commented out in favor of throwing an exception. I have not been able to find any case that currently works that is broken by this code change.

I have a patch in the works (with a bunch of tests) to fix this in the above mentioned trivial fashion, but I am currently awaiting response on if I can sign the CLA from employer.

@havocp
Copy link
Collaborator

havocp commented Jan 23, 2020

Is this the same as #332 ? If so there's some discussion on that issue from the last time I looked into it, with the punchline suggesting the same PR you came up with, but maybe you can read what it says on that issue and double-check that it matches your understanding.

@liljencrantz
Copy link
Author

It's not the exact same, e.g. allowUnresolved is set to true in my example, and you use optional substitutions, I use mandatory ones. But yeah, these issues are clearly related.

Didn't see it when looking for older issues. My bad.

A few comments on your comment, though. The ResolveContext that throws the error didn't fail to maintain it's root path, it does not need a path as it is the root. It gets created in ResolveContext.java:227. That constructor will create the object with the path set to null, because we are at the root, so there is no path. Null in a linked list means an empty list.

@liljencrantz
Copy link
Author

If you put the node inside another node, everything works.

E.g. {a:{b: ${c}}} does work as expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants