search

phax / ph-commons

Java 11 Library with tons of utility classes required in all projects
Apache License 2.0
30 stars 18 forks source link

XMLHelper.getPathToNode returns incorrect position number #31

Closed bertrand-lorentz closed 1 year ago

bertrand-lorentz commented 1 year ago

In XMLHelper.getPathToNode, if a node that is part of the path has one or more siblings with the same name, the path returned indicates the position of the node : /a/b[2]/c

But the position number seems to be off by one. For example, if the node is the first sibling, the position will be 0. But with XPath, position numbers start at 1

This is visible in the unit test at https://github.com/phax/ph-commons/blob/master/ph-xml/src/test/java/com/helger/xml/XMLHelperTest.java#L319 The node passed to XMLHelper.getPathToNode is the second occurrence of "e1" : //e1[2] But the location returned indicates it as .../e1[1]

This also affects XMLHelper.getPathToNode2, and this in turn affects ph-schematron in pure mode: the location attribute for failed-assert in SVRL validation reports is off by one.

If this is indeed an issue and not the intended behavior, I can submit a pull request to correct this.

phax commented 1 year ago

Hi @bertrand-lorentz thanks for pointing that out. This is indeed an issue.

That is of course an issue. I will investigate that further and see how to resolve this as backwards compatible as possible.

I assume you refer to the usage in PSXPathValidationHandlerSVRL._getPathToNode - correct?

bertrand-lorentz commented 1 year ago

Thanks for the quick response.

I assume you refer to the usage in PSXPathValidationHandlerSVRL._getPathToNode - correct?

Yes, it's that method that lead me to XMLHelper.getPathToNode2. Please note I was only looking at the code, I haven't traced or debugged it. But after finding the unit test, I thought it best to file this issue.

phax commented 1 year ago

I heavily increased the "path to node" functionality to also handle attributes and namespaces etc. This is part of the 10.2.2 release

phax commented 1 year ago

@bertrand-lorentz I created a new ph-schematron release 6.3.4 for this: https://github.com/phax/ph-schematron/releases/tag/ph-schematron-parent-pom-6.3.4 hth

bertrand-lorentz commented 1 year ago

Excellent, I just confirmed that the issue is fixed with ph-schematron 6.3.4. Thanks !