This was just wrong in the new setup; we change the root constantly.
The MemoKey should be per-conceptual-resolve which is per-ResolveContext,
not per-root.
This commit fixes the tests in which we failed to memoize
and thus got different results for the same ConfigReference
at different times in the resolution process.
But it breaks ConfigSubstitutionTest.avoidDelayedMergeObjectResolveProblem5.
This makes the typical path parse much faster:
- avoid Character.isLetter in favor of just ASCII alphanumeric
- allow fast path with hyphens and underscores involved
- don't use PathBuilder, build a Path directly by
traversing the string backward; this avoids some
object allocation
The immediate motivation here was to fix#177, which this does,
but in this commit a couple of existing test cases are broken
in a way which seems to relate to order of resolution and resolve
memoization. So we need to layer on to this commit better solutions
for caching and cycle detection to get rid of yet more mutable state.
The previous setup used a side-effect-based lookup table of "replacement"
values to conceptually modify the tree without actually modifying it.
Unfortunately that setup was hacky and hard to reason about and,
apparently, broken in cases such as #177.
This new setup actually creates a modified tree and passes it
around explicitly instead of inside ResolveContext.
In this commit, ResolveContext still unfortunately has a mutable
cache and a mutable table of "cycle markers." Both of those
in theory could also be replaced by simply modifying the tree.
The main downside to this commit - and to cleaning up the remaining
mutable state - is that we're using Java collections which have to
be copied wholesale for every mutation (they are not persistent
functional data structures). This will have an unknown performance
impact, though in a sane world Config.resolve() is not a bottleneck in
anyone's production app.
Some other details of this commit:
* resolve concerns removed from peekPath in AbstractConfigObject
and relocated into ResolveSource
* recursive resolution removed from lookupSubst and moved to
ConfigReference
* new hasDescendant() method used only in debug tracing,
it is grossly inefficient to ever call this full tree
traversal
* new replaceChild() method is inefficient due to Java
collections but could in theory be made efficient
* most complexity relates to always knowing the parent of
a node that we might have to replace, so we can walk
up replacing it in its ancestor chain
TODO in subsequent commits:
* fix failing test cases
* we cannot replaceChild if we are a descendant of ConfigConcatenation,
but we probably (?) need to be able to; consider / fix this
* instead of memoizing resolve results in a hash table, just
continuously modify the ResolveSource to have the most recent
results
* instead of using the "cycle markers" table, change the
ConfigReference to a cycle detector value
This is a spec change but the spec was wrong and the implementation
previously just threw an exception.
The spec claimed that ${?foo}${?bar} would become empty string,
but it's more useful for it to just become undefined.
You can add an explicit empty string with "" if you want it
to be empty string.
Fixes#161.
Fixes#155. Really this is kind of a mess; having overloads
with both ClassLoader and ConfigParseOptions parameters, when
the parse options may include a class loader, confuses the
heck out of things.
Trying to make it so that the ConfigParseOptions variants are
the canonical version, and the ones with a loader just immediately
set the loader on the parse options and call the variant with no
loader parameter.
Anyway ... when it fails this patch should at least make it much
clearer what went wrong.
This arose in issue https://github.com/typesafehub/config/issues/160
There are two known cases: when we want to relativize references
in included files, and when we use `+=` while inside a list.
Both of those are currently impossible to handle without a way
to generate a reference to a list element.
