Skip to content

[JXPATH-204] Eagerly evaluate UnionContext before returning its value - #289

Open
akashchamp wants to merge 1 commit into
apache:masterfrom
akashchamp:JXPATH-204-union-nodeset
Open

akashchamp wants to merge 1 commit into
apache:masterfrom
akashchamp:JXPATH-204-union-nodeset

Conversation

@akashchamp

@akashchamp akashchamp commented Sep 24, 2026 •

Copy link
Copy Markdown

Fixes JXPATH-204.

Problem

UnionContext (the EvalContext produced for a union expression like a | b)
only populates its NodeSet lazily, inside setPosition(). Normal XPath
evaluation always calls setPosition()/nextNode() before reading values, so
this is invisible in most usages.

However, ExtensionFunction.computeValue() resolves each argument by calling
getValue() on the argument's EvalContext directly, without iterating it
first:

parameters[i] = convert(args[i].compute(context));
...
private Object convert(final Object object) {
    return object instanceof EvalContext ? ((EvalContext) object).getValue() : object;
}

EvalContext.getValue() delegates to getNodeSet(), and
NodeSetContext.getNodeSet() just returns the backing field as-is (it does
not iterate). Since UnionContext never had setPosition() invoked yet at
that point, the backing NodeSet is still empty. The net effect: a custom
extension function invoked with a union expression as an argument, e.g.
my:fn(a | b), receives an empty NodeSet instead of the union of a
and b.

Fix

Override getValue() in UnionContext to force the node list to be computed
(via getContextNodeList(), which is what setPosition()-driven iteration
would have done anyway) before delegating to super.getValue(). This matches
the fix proposed by the reporter in the JIRA issue.

@Override
public Object getValue() {
    getContextNodeList();
    return super.getValue();
}

Testing

  • Added ExtensionFunctionTest.testUnionOperatorArgument(), which calls a
    NodeSet-consuming extension function (test:countPointers) with a union
    of two location paths (/beans[1] | /beans[2]) and asserts both nodes are
    seen (2), where previously it saw 0.
  • Ran the new test against the unmodified UnionContext first to confirm
    it reproduces the reported bug (expected: <2> but was: <0>), then applied
    the fix and confirmed it passes.
  • Full suite: mvn test → 416 tests run, 0 failures, 0 errors (1 pre-existing
    skip, unrelated to this change).
  • mvn checkstyle:check pmd:check → clean.
  • mvn verify → success.
  • Manual runtime check outside the test framework: a small standalone program
    registering a custom int countPointers(NodeSet) extension function and
    evaluating my:countPointers(/beans[1] | /beans[2]) via JXPathContext
    printed 0 (bug) against the unfixed jar and 2 (correct) against the
    fixed jar, confirming the fix holds at the public API level, not just
    inside the unit test.

Checklist (from PR template)

  • Read the contribution guidelines for this project.
  • Read the ASF Generative Tooling Guidance.
  • AI was used to help prepare this pull request: Claude Code (Claude Sonnet 5) was used to investigate the report, write the fix and regression test, and run the build/tests described above. The diagnosis and the exact fix shape were laid out by the reporter in the JIRA issue; the AI tool implemented, tested, and verified it.
  • Run a successful build using the default Maven goal (mvn).
  • Write unit tests that match behavioral changes, where the tests fail if the changes to the runtime are not applied.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Each commit has a meaningful subject line and body.

UnionContext only populates its NodeSet lazily, inside setPosition().
A caller that reads getValue() without first iterating the context
(for example, an extension function invoked with a union expression,
e.g. test:fn(a | b), as one of its arguments) received an empty
NodeSet instead of the union of the operand node-sets.

Override getValue() to force the node set to be built via
getContextNodeList() before delegating to the parent implementation,
per the fix suggested in the JIRA report.

Adds a regression test in ExtensionFunctionTest that invokes a custom
NodeSet-consuming function with a union of two location paths and
asserts the full pointer count is seen.

Generated-by: Claude Sonnet 5 (Claude Code)
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

Successfully merging this pull request may close these issues.

1 participant