lists did not respond well to cases where only some of their
elements were resolved or even when none of them were resolved
but one of their object identities happened to change. Previously
this would throw a nasty 'bug or broken' exception so it seems
unlikely to have unintended side effects.
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
