thomasmueller commented on code in PR #2902:
URL: https://github.com/apache/jackrabbit-oak/pull/2902#discussion_r3256825575
##########
oak-run-commons/src/main/java/org/apache/jackrabbit/oak/index/indexer/document/tree/TreeStoreNodeState.java:
##########
@@ -201,7 +205,15 @@ private void fetch() {
if (index < 0) {
throw new IllegalArgumentException(key);
}
- current = key.substring(index + 1);
+ if (!key.startsWith(path + "\t")) {
Review Comment:
> If I understand correctly this is a stop condition because we've reached
the sibling following the current node ?
Yes. The bug is that a node was returned that is not a child node.
> So treeStore.getSession().iterator(path) returns all the paths starting a
path depth first ?
It doesn't use depth-first; it returns the entries in key order. I have now
updated the documentation.
> So the method should really be named
getDescendantNodeNamesIterator(String path)
The methods is _supposed_ to return only the direct children. But it
returned non-children in some specific cases. This PR is supposed to fix that,
so it only returns direct children.
##########
oak-run-commons/src/main/java/org/apache/jackrabbit/oak/index/indexer/document/tree/TreeStoreNodeState.java:
##########
Review Comment:
Sure, this could be done, but I don't think it would make the code easier to
understand. I think whether or not a separate class should be used here is an
issue that is unrelated to his PR.
> Cognitive Complexity
For this case, I don't agree with Sonar that this method is too complex and
should be split into multiple methods.
--
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]
