nfsantos commented on code in PR #1597:
URL: https://github.com/apache/jackrabbit-oak/pull/1597#discussion_r1689545995
##########
oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/PathUtils.java:
##########
@@ -374,6 +374,32 @@ public static boolean isAncestor(String ancestor, String
path) {
return path.startsWith(ancestor);
}
+ /**
+ * Check if a path is a direct ancestor of another path.
+ *
+ * @param ancestor the ancestor path
+ * @param path the potential direct offspring path
+ * @return true if the path is a direct offspring of the ancestor
+ */
+ public static boolean isDirectAncestor(String ancestor, String path) {
+ assert isValid(ancestor) : "Invalid parent path [" + ancestor + "]";
+ assert isValid(path) : "Invalid path [" + path + "]";
+ if (ancestor.isEmpty() || path.isEmpty()) {
+ return false;
+ }
+ // The root is never a child of another path
+ if (PathUtils.denotesRoot(path)) {
+ return false;
+ }
+ int idx = path.lastIndexOf('/');
+ if (idx == 0) {
+ // path is a top level path. So it is only an immediate child of
root
+ return PathUtils.denotesRoot(ancestor);
+ } else {
+ return idx == ancestor.length() && path.regionMatches(0, ancestor,
0, idx);
+ }
+ }
+
Review Comment:
The proposed implementation would fail with isDirectAncestor("/", "/foo").
- `isAncestor(ancestor, path) -> true`
- `path.lastIndexOf('/')` = 0
- `ancestor.length() = 1`
We could add another condition to check if the ancestor is `/`, for
instance:
```
if (isAncestor(ancestor, path)) {
int idx = path.lastIndexOf('/');
if (idx == 0) {
return PathUtils.denotesRoot(ancestor);
} else {
return idx == ancestor.length();
}
} else {
return false;
}
```
But this has almost the same complexity as the standalone implementation
with the disadvantage of relying on a separate method.
--
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]
