Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add removeRecursive and listSubTreeBFS methods #88

Merged
merged 3 commits into from
Sep 18, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 54 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,10 @@ This module has been tested to work with ZooKeeper version 3.4.*.
+ [close](#void-close)
+ [create](#void-createpath-data-acls-mode-callback)
+ [remove](#void-removepath-version-callback)
+ [removeAll](#void-removeallpath-version-callback)
+ [exists](#void-existspath-watcher-callback)
+ [getChildren](#void-getchildrenpath-watcher-callback)
+ [getAllChildren](#void-getallchildrenpath-callback)
+ [getData](#void-getdatapath-watcher-callback)
+ [setData](#void-setdatapath-data-version-callback)
+ [getACL](#void-getaclpath-callback)
Expand Down Expand Up @@ -277,6 +279,31 @@ zookeeper.remove('/test/demo', -1, function (error) {

---

#### void removeAll(path, [version], callback)

Deletes a node and all its children with the given path and version.

**Arguments**

* path `String` - Path of the node.
* version `Number` - The version of the node, optional, defaults to -1.
* callback(error) `Function` - The callback function.

**Example**

```javascript
zookeeper.removeAll('/test/demo', -1, function (error) {
if (error) {
console.log(error.stack);
return;
}

console.log('Nodes removed.');
});
```

---

#### void exists(path, [watcher], callback)

Check the existence of a node. The callback will be invoked with the
Expand Down Expand Up @@ -346,6 +373,33 @@ zookeeper.getChildren('/test/demo', function (error, children, stats) {
});
```

---

#### void getAllChildren(path, callback)

Retrieve a list of all children including itself for the given node path.

**Arguments**

* path `String` - Path of the node.
* callback(error, children) `Function` - The callback function. The
children is an array of strings.

**Example**

```javascript
zookeeper.getAllChildren('/test/demo', function (error, children) {
if (error) {
console.log(error.stack);
return;
}

console.log('Children are: %j.', children);
});
```

---

#### void getData(path, [watcher], callback)

Retrieve the data and the stat of the node of the given path. If the watcher
Expand Down
20 changes: 20 additions & 0 deletions examples/getAllChildren.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
var zookeeper = require('../index.js');

var client = zookeeper.createClient(process.argv[2]);
var path = process.argv[3];

client.once('connected', function () {
console.log('Connected to the server.');

client.getAllChildren(path, function (error, children) {
if (error) {
console.log('Failed to list all child nodes of %s due to:', path, error);
return;
}
console.log('All child nodes of %s are: %j', path, children);

client.close();
});
});

client.connect();
20 changes: 20 additions & 0 deletions examples/removeAll.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
var zookeeper = require('../index.js');

var client = zookeeper.createClient(process.argv[2]);
var path = process.argv[3];

client.once('connected', function () {
console.log('Connected to the server.');

client.removeAll(path, function (error) {
if (error) {
console.log('Failed to remove all nodes for %s: %s', path, error);
} else {
console.log('Removed all nodes for: %s', path);
}

client.close();
});
});

client.connect();
78 changes: 78 additions & 0 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,45 @@ Client.prototype.remove = function (path, version, callback) {
);
};

/**
* Deletes a node and all its children.
*
* @param path {String} The node path.
* @param [version=-1] {Number} The version of the node.
* @param callback {Function} The callback function.
*/
Client.prototype.removeAll = function(path, version, callback) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the ZKUtil names better: removeRecursively, which makes it clear that it will remove the provided root node as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's currently named deleteRecursive (https://github.com/apache/zookeeper/blob/master/zookeeper-server/src/main/java/org/apache/zookeeper/ZKUtil.java#L51) but to keep the remove naming convention of this library, should I change to removeRecursive?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renamed to removeRecursive and listSubTreeBFS

if (!callback) {
callback = version;
version = -1;
}

Path.validate(path);

assert(typeof callback === 'function', 'callback must be a function.');
assert(typeof version === 'number', 'version must be a number.');

var self = this;

self.getAllChildren(path, function (error, children) {
if (error) {
callback(error);
return;
}
async.eachSeries(children.reverse(), function (nodePath, next) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reverse is called here to remove leaf nodes first

self.remove(nodePath, version, function(err) {
// Skip NO_NODE exception
if (err && err.getCode() === Exception.NO_NODE) {
next(null);
return;
}

next(err);
});
}, callback);
});
};

/**
* Set the data for the node of the given path if such a node exists and the
* optional given version matches the version of the node (if the given
Expand Down Expand Up @@ -814,6 +853,45 @@ Client.prototype.getChildren = function (path, watcher, callback) {
);
};

/**
* BFS list of the system under path. Note that this is not an atomic snapshot of
* the tree, but the state as it exists across multiple RPCs from clients to the
* ensemble.
*
* @method getAllChildren
* @param path {String} The node path.
* @param callback {Function} The callback function.
*/
Client.prototype.getAllChildren = function(path, callback) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to expose this method? Again, the the listSubTreeBFS is a better name even for private method.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's also a public method on ZKUtil. I'll rename this to listSubTreeBFS.

Path.validate(path);
assert(typeof callback === 'function', 'callback must be a function.');

var self = this;
var tree = [path];
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This includes the path that was passed in as part of the results, which is also what ZKUtil.listSubTreeBFS seems to do.


async.reduce(tree, tree, function(memo, item, next) {
self.getChildren(item, function (error, children) {
if (error) {
next(error);
return;
}
if (!children || !Array.isArray(children) || !children.length) {
next(null, tree);
return;
}
children.forEach(function(child) {
var childPath = item + '/' + child;

if (item === '/') {
childPath = item + child;
}
tree.push(childPath);
});
next(null, tree);
});
}, callback);
};

/**
* Create node path in the similar way of `mkdir -p`
*
Expand Down