akashchamp opened a new pull request, #289: URL: https://github.com/apache/commons-jxpath/pull/289
Fixes [JXPATH-204](https://issues.apache.org/jira/browse/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: ```java 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. ```java @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) - [x] Read the [contribution guidelines](CONTRIBUTING.md) for this project. - [x] Read the [ASF Generative Tooling Guidance](https://www.apache.org/legal/generative-tooling.html). - [x] AI was used to help prepare this pull request: [Claude Code](https://claude.com/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. - [x] Run a successful build using the default Maven goal (`mvn`). - [x] Write unit tests that match behavioral changes, where the tests fail if the changes to the runtime are not applied. - [x] Write a pull request description that is detailed enough to understand what the pull request does, how, and why. - [x] Each commit has a meaningful subject line and body. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
