junkaixue commented on code in PR #2460:
URL: https://github.com/apache/helix/pull/2460#discussion_r1180550227
##########
zookeeper-api/src/main/java/org/apache/helix/zookeeper/zkclient/util/ZkPathRecursiveWatcherTrie.java:
##########
@@ -139,17 +139,88 @@ public void addRecursiveListener(final String path,
RecursivePersistListener lis
if (path.isEmpty()) {
throw new IllegalArgumentException("Empty path: " + path);
}
- final List<String> pathComponents = split(path);
+ final List<String> pathComponents = split(path);
- synchronized(this) {
+ synchronized (this) {
TrieNode parent = _rootNode;
for (final String part : pathComponents) {
- parent = parent.getChildren().computeIfAbsent(part, (p)-> new
TrieNode(part) );
+ parent = parent.getChildren().computeIfAbsent(part, (p) -> new
TrieNode(part));
}
parent._recursiveListeners.add(listener);
}
}
+ public Set<RecursivePersistListener> getAllRecursiveListeners(String path) {
+ Objects.requireNonNull(path, "Path cannot be null");
+
+ final List<String> pathComponents = split(path);
+ Set<RecursivePersistListener> result = new HashSet<>();
+ synchronized (this) {
+ TrieNode cur = _rootNode;
+ for (final String element : pathComponents) {
+ cur = cur.getChild(element);
+ if (cur == null) {
+ break;
+ }
+ result.addAll(cur.getRecursiveListeners());
+ }
Review Comment:
This piece of code can be generalized:
synchronized (this) {
TrieNode cur = _rootNode;
for (final String element : pathComponents) {
cur = cur.getChild(element);
if (cur == null) {
break;
}
callable.call(cur);
}
For this call() {
result.addAll(cur.getRecursiveListeners());
}
##########
zookeeper-api/src/main/java/org/apache/helix/zookeeper/zkclient/util/ZkPathRecursiveWatcherTrie.java:
##########
@@ -139,17 +139,88 @@ public void addRecursiveListener(final String path,
RecursivePersistListener lis
if (path.isEmpty()) {
throw new IllegalArgumentException("Empty path: " + path);
}
- final List<String> pathComponents = split(path);
+ final List<String> pathComponents = split(path);
- synchronized(this) {
+ synchronized (this) {
TrieNode parent = _rootNode;
for (final String part : pathComponents) {
- parent = parent.getChildren().computeIfAbsent(part, (p)-> new
TrieNode(part) );
+ parent = parent.getChildren().computeIfAbsent(part, (p) -> new
TrieNode(part));
}
parent._recursiveListeners.add(listener);
}
}
+ public Set<RecursivePersistListener> getAllRecursiveListeners(String path) {
+ Objects.requireNonNull(path, "Path cannot be null");
+
+ final List<String> pathComponents = split(path);
+ Set<RecursivePersistListener> result = new HashSet<>();
+ synchronized (this) {
+ TrieNode cur = _rootNode;
+ for (final String element : pathComponents) {
+ cur = cur.getChild(element);
+ if (cur == null) {
+ break;
+ }
+ result.addAll(cur.getRecursiveListeners());
+ }
+ }
+
+ return result;
+ }
+
+ /**
+ * Removing a RecursivePersistWatcherListener on a path.
+ *
+ * Delete a path from the nearest trie node to current node if this is the
only listener and there
+ * is no child on the trie node.
+ *
+ * @param path the of the lister registered
+ * @param listener the RecursivePersistListener to be removed
+ */
+ public void removeRecursiveListener(final String path,
RecursivePersistListener listener) {
+ Objects.requireNonNull(path, "Path cannot be null");
+
+ if (path.length() == 0) {
+ throw new IllegalArgumentException("Invalid path: " + path);
+ }
+ final List<String> pathComponents = split(path);
+
+ synchronized (this) {
+ TrieNode cur = _rootNode;
+ TrieNode highestNodeForDelete =
+ null; // track the highest node that from that node to leaf node.
+ TrieNode prevDeletable = _rootNode;
+ for (final String part : pathComponents) {
+ cur = cur.getChild(part);
+ if (cur == null) {
+ return;
+ }
+
+ boolean candidateToDelete =
+ (cur.getChildren().size() == 1 &&
cur.getRecursiveListeners().size() == 0) || (
+ cur.getChildren().size() == 0 &&
cur.getRecursiveListeners().size() == 1 && cur
+ .getRecursiveListeners().contains(listener));
+ if (candidateToDelete) {
+ if (highestNodeForDelete == null) {
+ highestNodeForDelete = cur;
+ }
+ } else {
+ prevDeletable = cur;
+ highestNodeForDelete = null;
+ }
+ }
+ if (!cur.getRecursiveListeners().contains(listener)) {
+ return;
+ }
+ cur.getRecursiveListeners().remove(listener);
+
+ if (highestNodeForDelete != null) {
+ prevDeletable.getChildren().remove(highestNodeForDelete.getValue());
+ }
Review Comment:
This will be generalized as the call() {
xxx
}
if (!cur.getRecursiveListeners().contains(listener)) {
return;
}
For this line, you have have a flag in original code, if flag == false, then
stop search.
##########
zookeeper-api/src/main/java/org/apache/helix/zookeeper/zkclient/util/ZkPathRecursiveWatcherTrie.java:
##########
@@ -139,17 +139,88 @@ public void addRecursiveListener(final String path,
RecursivePersistListener lis
if (path.isEmpty()) {
throw new IllegalArgumentException("Empty path: " + path);
}
- final List<String> pathComponents = split(path);
+ final List<String> pathComponents = split(path);
- synchronized(this) {
+ synchronized (this) {
TrieNode parent = _rootNode;
for (final String part : pathComponents) {
- parent = parent.getChildren().computeIfAbsent(part, (p)-> new
TrieNode(part) );
+ parent = parent.getChildren().computeIfAbsent(part, (p) -> new
TrieNode(part));
}
parent._recursiveListeners.add(listener);
}
}
+ public Set<RecursivePersistListener> getAllRecursiveListeners(String path) {
+ Objects.requireNonNull(path, "Path cannot be null");
+
+ final List<String> pathComponents = split(path);
+ Set<RecursivePersistListener> result = new HashSet<>();
+ synchronized (this) {
+ TrieNode cur = _rootNode;
+ for (final String element : pathComponents) {
+ cur = cur.getChild(element);
+ if (cur == null) {
+ break;
+ }
+ result.addAll(cur.getRecursiveListeners());
+ }
+ }
+
+ return result;
+ }
+
+ /**
+ * Removing a RecursivePersistWatcherListener on a path.
+ *
+ * Delete a path from the nearest trie node to current node if this is the
only listener and there
+ * is no child on the trie node.
+ *
+ * @param path the of the lister registered
+ * @param listener the RecursivePersistListener to be removed
+ */
+ public void removeRecursiveListener(final String path,
RecursivePersistListener listener) {
+ Objects.requireNonNull(path, "Path cannot be null");
+
+ if (path.length() == 0) {
+ throw new IllegalArgumentException("Invalid path: " + path);
+ }
+ final List<String> pathComponents = split(path);
+
+ synchronized (this) {
+ TrieNode cur = _rootNode;
+ TrieNode highestNodeForDelete =
+ null; // track the highest node that from that node to leaf node.
+ TrieNode prevDeletable = _rootNode;
+ for (final String part : pathComponents) {
+ cur = cur.getChild(part);
+ if (cur == null) {
+ return;
+ }
+
+ boolean candidateToDelete =
Review Comment:
Better add some explanation here. For the people does not have context may
get confused.
##########
zookeeper-api/src/main/java/org/apache/helix/zookeeper/zkclient/util/ZkPathRecursiveWatcherTrie.java:
##########
@@ -139,17 +139,88 @@ public void addRecursiveListener(final String path,
RecursivePersistListener lis
if (path.isEmpty()) {
throw new IllegalArgumentException("Empty path: " + path);
}
- final List<String> pathComponents = split(path);
+ final List<String> pathComponents = split(path);
- synchronized(this) {
+ synchronized (this) {
TrieNode parent = _rootNode;
for (final String part : pathComponents) {
- parent = parent.getChildren().computeIfAbsent(part, (p)-> new
TrieNode(part) );
+ parent = parent.getChildren().computeIfAbsent(part, (p) -> new
TrieNode(part));
}
parent._recursiveListeners.add(listener);
}
}
+ public Set<RecursivePersistListener> getAllRecursiveListeners(String path) {
+ Objects.requireNonNull(path, "Path cannot be null");
+
+ final List<String> pathComponents = split(path);
+ Set<RecursivePersistListener> result = new HashSet<>();
+ synchronized (this) {
+ TrieNode cur = _rootNode;
+ for (final String element : pathComponents) {
+ cur = cur.getChild(element);
+ if (cur == null) {
+ break;
+ }
+ result.addAll(cur.getRecursiveListeners());
+ }
+ }
+
+ return result;
+ }
+
+ /**
+ * Removing a RecursivePersistWatcherListener on a path.
+ *
+ * Delete a path from the nearest trie node to current node if this is the
only listener and there
+ * is no child on the trie node.
+ *
+ * @param path the of the lister registered
+ * @param listener the RecursivePersistListener to be removed
+ */
+ public void removeRecursiveListener(final String path,
RecursivePersistListener listener) {
+ Objects.requireNonNull(path, "Path cannot be null");
+
+ if (path.length() == 0) {
+ throw new IllegalArgumentException("Invalid path: " + path);
+ }
+ final List<String> pathComponents = split(path);
+
+ synchronized (this) {
+ TrieNode cur = _rootNode;
+ TrieNode highestNodeForDelete =
+ null; // track the highest node that from that node to leaf node.
+ TrieNode prevDeletable = _rootNode;
+ for (final String part : pathComponents) {
+ cur = cur.getChild(part);
+ if (cur == null) {
+ return;
+ }
+
+ boolean candidateToDelete =
+ (cur.getChildren().size() == 1 &&
cur.getRecursiveListeners().size() == 0) || (
Review Comment:
This condition means this node has only one subpath. If subpath contains
listener and removed, then why not remove this node itself. This node should be
empty as well.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
