Skip to content

[bugfix] fn:transform: Conversion and treeIndex problems #5680

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 23 commits into
base: develop-6.x.x
Choose a base branch
from

Conversation

nverwer
Copy link

@nverwer nverwer commented Mar 18, 2025

Description

The fn:transform function sometimes produced NullPointerExceptions. This happened for documents containing nodes of type org.exist.dom.memtree.ElementImpl and other subtypes of org.exist.dom.memtree.NodeImpl.

The function org.exist.xquery.functions.fn.transform.Convert.ToSaxon.ofNode(Node) uses org.exist.xquery.functions.fn.transform.TreeUtils.treeIndex(Node) to get a 'path' (as a list of indexes) of the Node in its containing document. This 'path' is used to find the node in a newly built document that is suitable for use by Saxon.
Using the 'path' that the treeIndex function produces assumes that it starts at the document node.

However, some nodes do not have a containing document. Specifically, in org.exist.dom.memtree.NodeImpl.getParentNode():

if (parent.getNodeType() == DOCUMENT_NODE && !((DocumentImpl) parent).isExplicitlyCreated()) {
            /*
                All nodes in the MemTree will return an Owner document due to how the MemTree is implemented,
                however the explicitlyCreated flag tells us whether there "really" was a Document Node or not.
                See https://github.com/eXist-db/exist/issues/1463
             */
            return null;

In this case, the 'path' returned by treeIndex does not contain the index (0) for the root node, and org.exist.xquery.functions.fn.transform.TreeUtils.xdmNodeAtIndex(XdmNode, List<Integer>) will return null when a node is expected.

The proposed fix tests for this case. Going up the ancestor chain, when a node that is not a document node, and that does not have a parent is found, the index 0 for this root element is inserted in the 'path'.

Testing

The NullPointerException happened in a rather complicated XQuery with documents composed from several sources.
I have not yet been able to make a simple test case, but I will keep trying.

duncdrum and others added 20 commits November 13, 2024 16:06
set :release to develop-6.x.x branch
[6.x.x] Repair JNLP interface, use correct BC library
- load the module from existdb instead of an external source
- test with and without xmldb: scheme
- fix template under test to actually do something
- ensure that the output is checked for correctness
When importing stylesheets the URIs are resolved in order

- registered import-uris in package repo
- XMLDB-locations (relative, absolute and with or without scheme)
- any other location is treated as an exteranal source

The test class and its cases were renamed to reflect their purpose.
Some inline comments were added to help describing the intent.
fixes eXist-db#5525
fixes eXist-db#5530

- add functx to autodeploy for xquery tests
- add tests for one-off queries with module imports
  - of a registered module without location hint
  - of a module with location hint
- change XQueryContext to allow imports again
- change SourceFactory to work with contextPath set to "."
keep :latest and :release tags aligned
add dev:6  tag for snapshots from develop-6.x.x

see eXist-db#3953
[6.x.x] allow module imports in one-off xqueries
[6.x.x] registered import-uris have precedence
@dizzzz
Copy link
Member

dizzzz commented Mar 18, 2025

@wolfgangmm please can you check?

@nverwer
Copy link
Author

nverwer commented Mar 20, 2025

It appears there is another (different) problem with the way fn:transform handles its input, see #5682.

@nverwer
Copy link
Author

nverwer commented Mar 21, 2025

I have added two more commits.

  • Show the location when an error occurs during an XSL transformation (because my client who uses fn:transform wanted this).
  • Fix an error when transforming a document containing 'NodeProxy` nodes into an XdmValue for Saxon.

The second commit fixes another bug in the handling of NodeProxy instances.

@nverwer
Copy link
Author

nverwer commented Mar 21, 2025

There was another problem with the the 'path' returned by treeIndex.
The indexes are determined by counting previous-siblings.
It turns out that a org.exist.dom.persistent.StoredNode returns attributes of an element as previous siblings of the element's children.
When later the tree is traversed using the indexes, attributes are not in the children list.

I think that what StoredNode does is wrong, but decided to not try to solve it there (yet).
Instead, I made a work-around in TreeUtils.

@nverwer nverwer changed the title [bugfix] fn:transform: treeIndex for nodes without document-node ancestor [bugfix] fn:transform: treeIndex problems Mar 21, 2025
adamretter added a commit to evolvedbinary/elemental that referenced this pull request Apr 13, 2025
@line-o line-o added the needs Junit test Java test required to reproduce label Apr 15, 2025
@line-o
Copy link
Member

line-o commented Apr 15, 2025

Thanks @nverwer again for your contribution. We will pull it in once we have at least one positive and one negative test for it.
Any pointers you could give how to test this scenario would be much appreciated.

@line-o
Copy link
Member

line-o commented Apr 15, 2025

Or maybe it is easier to test in XQuery?

adamretter added a commit to evolvedbinary/elemental that referenced this pull request Apr 15, 2025
adamretter added a commit to evolvedbinary/elemental that referenced this pull request Apr 15, 2025
adamretter added a commit to evolvedbinary/elemental that referenced this pull request Apr 21, 2025
adamretter added a commit to evolvedbinary/elemental that referenced this pull request Apr 22, 2025
adamretter added a commit to evolvedbinary/elemental that referenced this pull request Apr 22, 2025
adamretter added a commit to evolvedbinary/elemental that referenced this pull request Apr 22, 2025
adamretter added a commit to evolvedbinary/elemental that referenced this pull request Apr 23, 2025
adamretter added a commit to evolvedbinary/elemental that referenced this pull request Apr 23, 2025
adamretter added a commit to evolvedbinary/elemental that referenced this pull request Apr 23, 2025
adamretter added a commit to evolvedbinary/elemental that referenced this pull request Apr 24, 2025
adamretter added a commit to evolvedbinary/elemental that referenced this pull request Apr 24, 2025
adamretter added a commit to evolvedbinary/elemental that referenced this pull request Apr 24, 2025
adamretter added a commit to evolvedbinary/elemental that referenced this pull request Apr 24, 2025
adamretter added a commit to evolvedbinary/elemental that referenced this pull request Apr 24, 2025
adamretter added a commit to evolvedbinary/elemental that referenced this pull request Apr 24, 2025
adamretter added a commit to evolvedbinary/elemental that referenced this pull request Apr 24, 2025
adamretter added a commit to evolvedbinary/elemental that referenced this pull request Apr 24, 2025
adamretter added a commit to evolvedbinary/elemental that referenced this pull request Apr 24, 2025
adamretter added a commit to evolvedbinary/elemental that referenced this pull request Apr 25, 2025
adamretter added a commit to evolvedbinary/elemental that referenced this pull request Apr 25, 2025
adamretter added a commit to evolvedbinary/elemental that referenced this pull request Apr 25, 2025
adamretter added a commit to evolvedbinary/elemental that referenced this pull request Apr 26, 2025
adamretter added a commit to evolvedbinary/elemental that referenced this pull request Apr 26, 2025
adamretter added a commit to evolvedbinary/elemental that referenced this pull request May 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs develop port needs Junit test Java test required to reproduce
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants