Merge pull request #2 from coinbase/2.0.0

2.0.0

Author: CjS77

.gitignore

@@ -25,3 +25,5 @@ build/Release
 # Dependency directory
 # https://www.npmjs.org/doc/misc/npm-faq.html#should-i-check-my-node_modules-folder-into-git
 node_modules
+.DS_Store
+.idea/

package.json

@@ -1,6 +1,6 @@
 {
   "name": "process-lock-node",
-  "version": "1.0.1",
+  "version": "2.0.0",
   "description": "A very simple process locking module using redis",
   "main": "process_lock.js",
   "directories": {
@@ -15,6 +15,7 @@
   },
   "author": "",
   "dependencies": {
-    "redis": "2.0.1"
+    "redis": "2.0.1",
+    "uuid": "3.1.0"
   }
 }

process_lock.js

@@ -1,4 +1,5 @@
 var redis = require('redis');
+var uuid = require('uuid/v4');
 var redisClient = redis.createClient(
   process.env.REDIS_URL
 );
@@ -28,10 +29,13 @@ var redisClient = redis.createClient(
 var ProcessLock = function(key, timeout) {
   this.key = key;
   this.timeout = timeout || 10;
-  this.id = process.pid;
+  this.id = uuid();
   this.owner = false;
 };
 
+/**
+ * Request a lock on a process. The callback is a standard node callback that returns true if the lock was obtained.
+ */
 ProcessLock.prototype.lock = function(callback) {
   var self = this;
 
@@ -41,46 +45,67 @@ ProcessLock.prototype.lock = function(callback) {
       self.owner = true;
     }
 
-    typeof callback === 'function' && callback();
+    typeof callback === 'function' && callback(err, self.owner);
   });
 };
 
-ProcessLock.prototype.clear = function(callback, force) {
+/**
+ * Clear the lock. If force is true, the lock clearing is forced. Otherwise, the lock is only cleared if this objects owns the lock.
+ * NB. As of version 2.0.0, the callback argument is last as per node conventions.
+ * If the lock was not cleared because we are not the owner, the callback returns false.
+ * The callback returns true if the lock was explicitly cleared and if the lock did not exist at all, the callback returns null
+ */
+ProcessLock.prototype.clear = function(force, callback) {
   var self = this;
-
+  if (typeof force === 'function') {
+    callback = force;
+    force = false;
+  }
   redisClient.get(self.key, function(err, value) {
-    if (value != self.id && !force) {
-      typeof callback === 'function' && callback();
+    if (value !== self.id && !force) {
+      // We have lost the lock
+      self.owner = false;
+      typeof callback === 'function' && callback(err, false);
       return;
     }
 
     redisClient.del(self.key, function(err, value) {
       self.owner = false;
-      typeof callback === 'function' && callback();
+      typeof callback === 'function' && callback(err, value === 1 ? true : null);
     });
   });
 };
 
+/**
+ * Try to renew the lock on the process. The callback will return false if a lock could not be renewed; true if it explicitly renewed the lock.
+ */
 ProcessLock.prototype.update = function(callback) {
   var self = this;
 
-  if (!self.owner) {
-    typeof callback === 'function' && callback();
-    return;
-  }
+  redisClient.get(self.key, function(err, value) {
+    // We have lost the lock
+    if (value != self.id) {
+      self.owner = false;
+    }
 
-  redisClient.expire(self.key, self.timeout);
+    if (!self.owner) {
+      typeof callback === 'function' && callback(err, false);
+      return;
+    }
 
-  typeof callback === 'function' && callback();
-}
+    redisClient.expire(self.key, self.timeout, (err, val) => {
+        typeof callback === 'function' && callback(err, val === 1);
+    });
+  });
+};
 
 ProcessLock.prototype.state = function(callback) {
   var self = this;
 
   redisClient.get(self.key, function(err, value) {
     value === null ? value = 'open' : value = 'locked';
-    callback(value);
+    callback(err, value);
   });
-}
+};
 
 module.exports = ProcessLock;

test/process_lock_test.js

@@ -1,89 +1,108 @@
-var assert = require('assert');
-var Process = require('./support/process');
+const assert = require('assert');
+const Process = require('./support/process');
 
-var firstProcess = new Process('same-name');
-var secondProcess = new Process('same-name');
+const firstProcess = new Process('same-name', 1);
+const secondProcess = new Process('same-name', 1);
 
 describe('ProcessLock', function() {
-  it('firstProcess should be able to obtain lock', function(done) {
-    firstProcess.processLock.lock(function(){
-      assert(firstProcess.processLock.owner);
-      done();
+    it('firstProcess should be able to obtain lock', function(done) {
+        firstProcess.processLock.lock((err, result) => {
+            assert.ifError(err);
+            assert(firstProcess.processLock.owner);
+            assert.equal(result, true);
+            done();
+        });
     });
-  });
 
-  it('secondProcess should not be able to obtain the lock', function(done) {
-    secondProcess.processLock.lock(function() {
-      assert.equal(false, secondProcess.processLock.owner);
-      done();
+    it('secondProcess should not be able to obtain the lock', function(done) {
+        secondProcess.processLock.lock((err, result) => {
+            assert.ifError(err);
+            assert.equal(result, false);
+            assert.equal(false, secondProcess.processLock.owner);
+            done();
+        });
     });
-  });
 
-  it('firstProcess should be able to let go of the lock', function(done) {
-    firstProcess.processLock.clear(function() {
-      assert.equal(false, firstProcess.processLock.owner);
-      done();
+    it('firstProcess should be able to let go of the lock', function(done) {
+        firstProcess.processLock.clear((err, result) => {
+            assert.ifError(err);
+            assert.equal(result, true);
+            assert.equal(false, firstProcess.processLock.owner);
+            done();
+        });
     });
-  });
 
-  it('secondProcess should be able to obtain the lock', function(done) {
-    secondProcess.processLock.lock(function() {
-      assert.equal(true, secondProcess.processLock.owner);
-      done();
+    it('secondProcess should be able to obtain the lock', function(done) {
+        secondProcess.processLock.lock((err, result) => {
+            assert.ifError(err);
+            assert.equal(secondProcess.processLock.timeout, 1);
+            assert.equal(result, true);
+            assert.equal(true, secondProcess.processLock.owner);
+            done();
+        });
     });
-  });
 
-  it('secondProcess should be able to update the lock', function(done) {
-    this.timeout(7000);
-
-    setTimeout(function() {
-      secondProcess.processLock.update(function() {
-        assert(secondProcess.processLock.owner);
-        done();
-      });
-    }, 5000)
-  });
-
-  it('firstProcess should not be able to update the lock', function(done) {
-    this.timeout(5000);
-
-    setTimeout(function() {
-      firstProcess.processLock.update(function() {
-        assert.equal(false, firstProcess.processLock.owner);
-        done();
-      });
-    }, 2000);
-  });
-
-  it('firstProcess should not be able to lock because secondProcess recently updated', function(done) {
-    this.timeout(15000);
+    it('secondProcess should be able to update the lock', function(done) {
+        setTimeout(() => {
+            secondProcess.processLock.update((err, result) => {
+                assert.ifError(err);
+                assert.equal(result, true);
+                assert(secondProcess.processLock.owner);
+                done();
+            });
+        }, 500)
+    });
 
-    setTimeout(function() {
-      firstProcess.processLock.lock(function() {
-        assert.equal(false, firstProcess.processLock.owner);
-        done();
-      });
-    }, 7000);
-  });
+    it('firstProcess should not be able to update the lock', function(done) {
+        setTimeout(() => {
+            firstProcess.processLock.update((err, result) => {
+                assert.ifError(err);
+                assert.equal(result, false);
+                assert.equal(false, firstProcess.processLock.owner);
+                done();
+            });
+        }, 500);
+    });
 
-  it('firstProcess should be able to lock due to timeout', function(done) {
-    this.timeout(12000)
+    it('firstProcess should not be able to lock because secondProcess recently updated', function(done) {
+        setTimeout(() => {
+            firstProcess.processLock.lock((err, result) => {
+                assert.ifError(err);
+                assert.equal(result, false);
+                assert.equal(false, firstProcess.processLock.owner);
+                done();
+            });
+        }, 250);
+    });
 
-    setTimeout(function() {
-      firstProcess.processLock.lock(function() {
-        assert(firstProcess.processLock.owner);
-        done();
-      });
-    }, 10000)
-  })
+    it('firstProcess should be able to lock due to timeout', function(done) {
+        setTimeout(() => {
+            firstProcess.processLock.lock((err, result) => {
+                assert.ifError(err);
+                assert.equal(result, true);
+                assert(firstProcess.processLock.owner);
+                done();
+            });
+        }, 1500)
+    });
 
-  it('secondProcess should be able to force the lock to clear', function(done) {
-    secondProcess.processLock.clear(function(){
-      secondProcess.processLock.state(function(state) {
-        assert.equal('open', state);
-        done();
-      })
-    }, true)
-  })
+    it('secondProcess should be able to force the lock to clear', function(done) {
+        secondProcess.processLock.clear(true, (err, result) => {
+            assert.ifError(err);
+            assert.equal(result, true);
+            secondProcess.processLock.state((err, state) => {
+                assert.ifError(err);
+                assert.equal('open', state);
+                done();
+            })
+        });
+    });
 
+    it('second lock clear returns null', (done) => {
+        secondProcess.processLock.clear(true, (err, result) => {
+            assert.ifError(err);
+            assert.equal(result, null);
+            done();
+        });
+    });
 });

test/support/process.js

@@ -1,8 +1,8 @@
 var ProcessLock = require('../../process_lock');
 
-var Process = function(name) {
+var Process = function(name, timeout) {
   this.name = name;
-  this.processLock = new ProcessLock(this.name, 10);
+  this.processLock = new ProcessLock(this.name, timeout);
 };
 
 module.exports = Process;