TEDEFO-5150: Preserve predicates and always return a valid path when adding an axis - #59
Merged
rousso merged 2 commits intoAug 29, 2026
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes TEDEFO-5150. Best merged squashed: the second commit replaces the design of the first rather than building on it.
XPathProcessor.addAxisdiscarded the predicates of the path it rewrote, and threw a bareNoSuchElementExceptionon a path made only of parent steps. Looking into it turned up further cases where it returned XPath that does not compile.What changed
Predicates are kept, by composing the result from
XPathStep.toString()rather than from the step text alone. This is the same fault as TEDEFO-5148, in a second function of the same class.The function now returns a valid path for every valid path given, and no longer throws:
b[x]/cpreceding::b/cpreceding::b[x]/c..[x]/bpreceding::bpreceding::node()[x]/b..NoSuchElementExceptionpreceding::node().preceding::.preceding::node()./bpreceding::./bpreceding::b@xpreceding::@xpreceding::node()/@xchild::b/cpreceding::child::b/cpreceding::b/cdoc('x')/bpreceding::doc('x')/bpreceding::node()/doc('x')/b/a/bpreceding::a/bpreceding::a/bLeading steps that only move about are dropped, as before, since an axis searches the document from the context node regardless of them; one carrying a predicate stays, because the predicate describes what we are looking for. Where the axis lands on a step that cannot be looked for elsewhere, that step keeps its place behind a step walking the axis.
An absolute path behaves exactly as it did. Its anchor cannot be kept, because an axis cannot be followed by a separator, so the path is read from the context as before.
How a step is read
Deciding what may follow an axis is read from the parse tree rather than guessed from the step text, which is what the first attempt did and what broke on
doc('x'),id('x')and(a | b). The listener asks the parser directly —KW_ATTRIBUTE(),KW_NAMESPACE(),KW_SELF(),KW_PARENT(), andkindtest().anykindtest()fornode()— so a step is read the same way however it is spelled:.andself::node()agree, as do..andparent::node(), andbandchild::b.What is recorded is only what
addAxisneeds, not a model of XPath: whether the step can be looked for along another axis, only moves about, or has to stay where it is. That classification and the node test behind it are private toXPathStep, and the original spelling of every step is preserved.No public API changes.
XPathStepkeeps its constructor, its runtime type, its equality and its ordering; a step read from a path and one built from its text remain equal, with agreeinghashCodeandcompareTo. The only entryjapicmpattributes to the class is thetoString()that came from TEDEFO-5148.Scope
addAxisserves the axis that can be written on an EFX-1 field reference and nothing more; it is not a general way of rewriting XPath, and its javadoc now says so. EFX-2 has no axis syntax andScriptGenerator.composeFieldReferenceWithAxisis already marked for removal, which is why the implementation is kept proportional to that one caller.Impact
No SDK content is affected. The only use of an axis in the SDK is
preceding::BT-687-LotResultin the view templates of notice types 32, 33 and 35, whose context already contains the predicated step, so the path reaching the function carries no predicate.Verification
Every output above was compiled with Saxon; a sweep of 30 inputs reports no valid input producing invalid output, and every abbreviated step was checked against its spelled-out form. The full test suite passes here,
japicmpreports binary compatibility, and all 1377 tests in the EFX toolkit pass against this build.