Fix #4 - create is always in PERSISTENT mode

alexguan · May 7, 2013 · 87211fc · 87211fc
1 parent 064712e
commit 87211fc
Show file tree

Hide file tree

Showing 7 changed files with 58 additions and 69 deletions.
diff --git a/examples/get.js b/examples/get.js
@@ -26,7 +26,7 @@ function getData(client, path) {
             console.log(
                 'Node: %s has data: %s, version: %d',
                 path,
-                data.toString(),
+                data ? data.toString() : undefined,
                 stat.version
             );
         }

diff --git a/examples/mkdirp.js b/examples/mkdirp.js
@@ -13,7 +13,7 @@ var path = process.argv[3];
 client.once('connected', function () {
     console.log('Connected to the server.');
 
-    client.mkdirp(path, function (error, p) {
+    client.mkdirp(path, zookeeper.CreateMode.PERSISTENT, function (error, p) {
         if (error) {
             console.log('Failed to mkdirp: %s due to: %s: ', path, error.stack);
         } else {

diff --git a/index.js b/index.js
@@ -244,59 +244,53 @@ Client.prototype.addAuthInfo = function (scheme, auth) {
  * @param callback {Function} The callback function.
  */
 Client.prototype.create = function (path, data, acls, mode, callback) {
-    assert(
-        arguments.length >= 2 && arguments.length <= 5,
-        'create needs at least the path and callback arguments.'
-    );
+    var optionalArgs = [data, acls, mode, callback],
+        header,
+        payload,
+        request;
 
     Path.validate(path);
-    callback = arguments[arguments.length - 1];
-
-    assert(
-        typeof callback === 'function',
-        'callback must be a function.'
-    );
-
-    acls = mode = data = undefined;
-
-    Array.prototype.slice.call(arguments).forEach(function (arg, i, args) {
-        // Skip the first and the last arguments
-        if (i === 0 || i === args.length - 1) {
-            return;
-        }
 
+    // Reset arguments so we can reassign correct value to them.
+    data = acls = mode = callback = undefined;
+    optionalArgs.forEach(function (arg, index) {
         if (Array.isArray(arg)) {
             acls = arg;
         } else if (typeof arg === 'number') {
             mode = arg;
         } else if (Buffer.isBuffer(arg)) {
             data = arg;
+        } else if (typeof arg === 'function') {
+            callback = arg;
         }
     });
 
+    assert(
+        typeof callback === 'function',
+        'callback must be a function.'
+    );
 
-    acls = acls || ACL.OPEN_ACL_UNSAFE;
+    acls = Array.isArray(acls) ? acls : ACL.OPEN_ACL_UNSAFE;
     mode = typeof mode === 'number' ? mode : CreateMode.PERSISTENT;
 
-    assert(acls.length > 0, 'acls must be a non-empty array.');
-
     assert(
         data === null || data === undefined || Buffer.isBuffer(data),
         'data must be a valid buffer, null or undefined.'
     );
+
     if (Buffer.isBuffer(data)) {
         assert(
             data.length <= DATA_SIZE_LIMIT,
             'data must be equal of smaller than ' + DATA_SIZE_LIMIT + ' bytes.'
         );
     }
 
-    var header = new jute.protocol.RequestHeader(),
-        payload = new jute.protocol.CreateRequest(),
-        request;
+    assert(acls.length > 0, 'acls must be a non-empty array.');
 
+    header = new jute.protocol.RequestHeader();
     header.type = jute.OP_CODES.CREATE;
 
+    payload = new jute.protocol.CreateRequest();
     payload.path = path;
     payload.acl = acls.map(function (item) {
         return item.toRecord();
@@ -309,7 +303,6 @@ Client.prototype.create = function (path, data, acls, mode, callback) {
     }
 
     request = new jute.Request(header, payload);
-
     this.connectionManager.queue(request, function (error, response) {
         if (error) {
             callback(error);
@@ -677,60 +670,55 @@ Client.prototype.getChildren = function (path, watcher, callback) {
  * @param callback {Function} The callback function.
  */
 Client.prototype.mkdirp = function (path, data, acls, mode, callback) {
-    assert(
-        arguments.length >= 2 && arguments.length <= 5,
-        'mkdirp needs at least the path and callback arguments.'
-    );
+    var optionalArgs = [data, acls, mode, callback],
+        self = this,
+        currentPath = '',
+        nodes;
 
     Path.validate(path);
-    callback = arguments[arguments.length - 1];
-
-    assert(
-        typeof callback === 'function',
-        'callback must be a function.'
-    );
-
-    data = acls = mode = undefined;
-
-    Array.prototype.slice.call(arguments).forEach(function (arg, i, args) {
-        // Skip the first and the last arguments
-        if (i === 0 || i === (args.length - 1)) {
-            return;
-        }
 
+    // Reset arguments so we can reassign correct value to them.
+    data = acls = mode = callback = undefined;
+    optionalArgs.forEach(function (arg, index) {
         if (Array.isArray(arg)) {
             acls = arg;
         } else if (typeof arg === 'number') {
             mode = arg;
         } else if (Buffer.isBuffer(arg)) {
             data = arg;
+        } else if (typeof arg === 'function') {
+            callback = arg;
         }
     });
 
+    assert(
+        typeof callback === 'function',
+        'callback must be a function.'
+    );
 
-    acls = acls || ACL.OPEN_ACL_UNSAFE;
+    acls = Array.isArray(acls) ? acls : ACL.OPEN_ACL_UNSAFE;
     mode = typeof mode === 'number' ? mode : CreateMode.PERSISTENT;
 
-    assert(acls.length > 0, 'acls must be a non-empty array.');
-
     assert(
         data === null || data === undefined || Buffer.isBuffer(data),
         'data must be a valid buffer, null or undefined.'
     );
+
     if (Buffer.isBuffer(data)) {
         assert(
             data.length <= DATA_SIZE_LIMIT,
             'data must be equal of smaller than ' + DATA_SIZE_LIMIT + ' bytes.'
         );
     }
 
-    var self = this,
-        nodes = path.split('/').slice(1), // Remove the empty string
-        currentPath = '';
+    assert(acls.length > 0, 'acls must be a non-empty array.');
+
+    // Remove the empty string
+    nodes = path.split('/').slice(1);
 
     async.eachSeries(nodes, function (node, next) {
         currentPath = currentPath + '/' + node;
-        self.create(currentPath, acls, mode, data, function (error, path) {
+        self.create(currentPath, data, acls, mode, function (error, path) {
             // Skip node exist error.
             if (error && error.getCode() === Exception.NODE_EXISTS) {
                 next(null);

diff --git a/lib/ConnectionManager.js b/lib/ConnectionManager.js
@@ -411,7 +411,9 @@ ConnectionManager.prototype.onSocketData = function (buffer) {
         // More data are coming.
         self.pendingBuffer = buffer;
         return;
-    } else if (buffer.length === size + 4) {
+    }
+
+    if (buffer.length === size + 4) {
         // The size is perfect.
         self.pendingBuffer = null;
     } else {

diff --git a/lib/Transaction.js b/lib/Transaction.js
@@ -47,20 +47,16 @@ function Transaction(connectionManager) {
  * @return {Transaction} this transaction instance.
  */
 Transaction.prototype.create = function (path, data, acls, mode) {
-    assert(
-        arguments.length >= 1 && arguments.length <= 4,
-        'create needs at least the path arguments.'
-    );
+    var optionalArgs = [data, acls, mode],
+        self = this,
+        currentPath = '',
+        nodes;
 
     Path.validate(path);
-    acls = mode = data = undefined;
-
-    Array.prototype.slice.call(arguments).forEach(function (arg, i, args) {
-        // Skip the first argument
-        if (i === 0) {
-            return;
-        }
 
+    // Reset arguments so we can reassign correct value to them.
+    data = acls = mode = undefined;
+    optionalArgs.forEach(function (arg, index) {
         if (Array.isArray(arg)) {
             acls = arg;
         } else if (typeof arg === 'number') {
@@ -70,13 +66,14 @@ Transaction.prototype.create = function (path, data, acls, mode) {
         }
     });
 
-    acls = acls || ACL.OPEN_ACL_UNSAFE;
+    acls = Array.isArray(acls) ? acls : ACL.OPEN_ACL_UNSAFE;
     mode = typeof mode === 'number' ? mode : CreateMode.PERSISTENT;
 
     assert(
         data === null || data === undefined || Buffer.isBuffer(data),
         'data must be a valid buffer, null or undefined.'
     );
+
     assert(acls.length > 0, 'acls must be a non-empty array.');
 
     this.ops.push({

diff --git a/lib/jute/index.js b/lib/jute/index.js
@@ -193,9 +193,9 @@ function prependChroot(self, path) {
 
     if (path === '/') {
         return self.chrootPath;
-    } else {
-        return self.chrootPath + path;
     }
+
+    return self.chrootPath + path;
 }
 
 /**

diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "node-zookeeper-client",
-  "version": "0.0.3",
+  "version": "0.0.4",
   "description": "A pure Javascript ZooKeeper client for Node.js.",
   "author": "Alex Guan <alex.guan@gmail.com>",
   "main": "index.js",
@@ -15,6 +15,7 @@
     "url": "https://github.com/alexguan/node-zookeeper-client"
   },
   "scripts": {
+    "pretest": "jslint --node --sloppy index.js lib/*.js lib/jute/*.js",
     "test": "mocha --recursive --reporter spec test/*",
     "coverage": "istanbul cover _mocha --recursive test/*"
   },
@@ -34,6 +35,7 @@
   "devDependencies": {
     "mocha": "~1.9.0",
     "chai": "~1.5.0",
-    "istanbul": "~0.1.34"
+    "istanbul": "~0.1.34",
+    "jslint": "~0.1.9"
   }
 }