diff --git a/.gitignore b/.gitignore index 123ae94..abdee5c 100644 --- a/.gitignore +++ b/.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/ diff --git a/package.json b/package.json index b391dad..7cc7db9 100644 --- a/package.json +++ b/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" } } diff --git a/process_lock.js b/process_lock.js index 2d9d6df..8450bae 100644 --- a/process_lock.js +++ b/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; diff --git a/test/process_lock_test.js b/test/process_lock_test.js index ed7f6b6..33c907f 100644 --- a/test/process_lock_test.js +++ b/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(); + }); + }); }); diff --git a/test/support/process.js b/test/support/process.js index 9b1132b..b509c9f 100644 --- a/test/support/process.js +++ b/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;