Details
-
Type:
Bug
-
Status:
Open
-
Priority:
Minor
-
Resolution: Unresolved
-
Affects Version/s: None
-
Fix Version/s: 2.0
-
Component/s: None
-
Labels:None
-
Number of attachments :
Description
Messing around with merging nodesets as I mentioned on the list , I ran into a problem that too many nodes were being returned. The structure used for the merge was new TreeSet(new NodeComparator(navigator)), which should have kept unique nodes in document order, but it turned out they werent
On inspection I found a number of problems with NodeComparator:
- it doesn't return 0 for equal nodes.
I added:
if (o1 == o2) { return 0; }at the start of 'compare' to resolve this.
This showed up a second problem with how attributes and namespace nodes are compared: they just compare as if they were their parent node. This isn't correct for two attributes of the name node:
if (isNonChild(o1) && isNonChild(o2)) {
try
catch (UnsupportedAxisException ex)
{ return 0; }}
I removed that code. NS/Attr node comparisons should identical to Element, PI, etc comparisons except for when they have the same parent node, so the special case needs to go in the 'compareSiblings' code. As I was a bit uncertain whether the following sibling check would work for ns/attr nodes I coded up their comparison explicitly:
int nodeType1 = navigator.getNodeType(sib1);
int nodeType2 = navigator.getNodeType(sib2);
if (nodeType1 == Pattern.ATTRIBUTE_NODE) {
if (nodeType2 == Pattern.NAMESPACE_NODE)
if (nodeType2 == Pattern.ATTRIBUTE_NODE)
{ String name1 = navigator.getAttributeQName(sib1); String name2 = navigator.getAttributeQName(sib2); int cmp = name1.compareTo(name2); return cmp; } return -1; // ie atttributes are before elements.
} else if (nodeType1 == Pattern.NAMESPACE_NODE) {
if (nodeType2 == Pattern.ATTRIBUTE_NODE)
if (nodeType2 == Pattern.NAMESPACE_NODE)
{ String name1 = navigator.getNamespacePrefix(sib1); String name2 = navigator.getNamespacePrefix(sib2); return name1.compareTo(name2); } return -1;
}
if (nodeType2 == Pattern.NAMESPACE_NODE
| nodeType2 == Pattern.ATTRIBUTE_NODE) { return 1; } |
|---|
this sorts namespace and attribute nodes separately and alphabetically, and passes the current tests. NB the use of getNodeType above triggers bug jaxen-101!
As for testing this code: if you test nodecomparator with two identical nodes it triggers the first problem, the rest of the issue shows up as test failures (for the order of attribute nodes on the ancestor axis, and the number of attribute/namespace nodes found) if you don't do the rest of the fix.
There may well be an issue here. Then again there may not be. I'll have to think more deeply about this.
NodeComparator is deliberately a non-public class. It does not attempt to handle every conceivable comparison of objects; only those that can actually arise through the public API. If you can construct a test case that demonstrates the alleged problems solely by invoking the public (or protected) APIs, then we'd have definitive proof that there's a bug here, and can begin fixing it. However, absent such a test case I'm not sure how to fix this, or how to be sure that any proposed fix really does fix the problem, or even whether a problem really exists.