Add tests

.jshintrc

@@ -1,5 +1,13 @@
 {
 	"node": true,
 	"unused": "vars",
-	"multistr": true
+	"multistr": true,
+	"globals": {
+		"describe": false,
+		"it": false,
+		"before": false,
+		"beforeEach": false,
+		"after": false,
+		"afterEach": false
+	}
 }

package.json

@@ -4,7 +4,7 @@
   "description": "A node.js redlock implementation for distributed redis locks",
   "main": "redlock.js",
   "scripts": {
-    "test": "echo \"Error: no test specified\" && exit 1"
+    "test": "mocha test.js"
   },
   "repository": {
     "type": "git",
@@ -24,5 +24,10 @@
     "url": "https://github.com/mike-marcacci/node-redlock/issues"
   },
   "homepage": "https://github.com/mike-marcacci/node-redlock",
+  "devDependencies": {
+    "chai": "^3.0.0",
+    "mocha": "^2.2.5",
+    "redis": "^0.12.1"
+  },
   "dependencies": {}
 }

redlock.js

@@ -1,8 +1,8 @@
 'use strict';
 
 // constants
-var unlockScript = 'if redis.call("get",KEYS[1]) == ARGV[1] then return redis.call("del",KEYS[1]) else return 0 end';
-var extendScript = 'if redis.call("get",KEYS[1]) == ARGV[1] then return redis.call("expire",ARGV[2]) else return 0 end';
+var unlockScript = 'if redis.call("get", KEYS[1]) == ARGV[1] then return redis.call("del", KEYS[1]) else return 0 end';
+var extendScript = 'if redis.call("get", KEYS[1]) == ARGV[1] then return redis.call("expire", KEYS[1], ARGV[2]) else return 0 end';
 
 // defaults
 var defaults = {
@@ -42,11 +42,11 @@ function Lock(redlock, resource, value, expiration) {
 	this.expiration = expiration;
 }
 
-Lock.prototype.unlock = function unlock(lock) {
-	this.redlock.unlock(this);
+Lock.prototype.unlock = function unlock(callback) {
+	this.redlock.unlock(this, callback);
 };
 
-Lock.prototype.extend = function extend(lock, ttl, callback) {
+Lock.prototype.extend = function extend(ttl, callback) {
 	this.redlock.extend(this, ttl, callback);
 };
 
@@ -114,16 +114,16 @@ Redlock.prototype.lock = function lock(resource, value, ttl, callback) {
 	if(typeof callback === 'undefined') {
 		callback = ttl;
 		ttl = value;
-		value = self.random();
+		value = self._random();
 		request = function(server, loop){
-			return server.set(resource, value, 'NX', 'PX', loop);
+			return server.set(resource, value, 'NX', 'PX', ttl, loop);
 		};
 	}
 
 	// extend an existing lock
 	else {
 		request = function(server, loop){
-			return server.eval(extendScript, 2, resource, value, ttl, loop);
+			return server.eval(extendScript, 1, resource, value, ttl, loop);
 		};
 	}
 
@@ -150,7 +150,7 @@ Redlock.prototype.lock = function lock(resource, value, ttl, callback) {
 		});
 
 		function loop(err, response) {
-			if(response) quorum++;
+			if(response) votes++;
 			if(waiting-- > 1) return;
 
 			// Add 2 milliseconds to the drift to account for Redis expires precision, which is 1 ms,
@@ -163,15 +163,20 @@ Redlock.prototype.lock = function lock(resource, value, ttl, callback) {
 				return callback(null, lock);
 
 			// remove this lock from servers that voted for it
-			if(votes < quorum)
-				lock.unlock();
+			// if(votes < quorum)
+			// 	return lock.unlock(next);
 
-			// RETRY
-			if(attempts < self.retryCount)
-				return setTimeout(attempt, self.retryDelay);
+			return next();
 
-			// FAILED
-			return callback(new LockError('Exceeded ' + self.retryCount + ' attempts to lock the resource "' + resource + '".'));
+			function next(){
+
+				// RETRY
+				if(attempts <= self.retryCount)
+					return setTimeout(attempt, self.retryDelay);
+
+				// FAILED
+				return callback(new LockError('Exceeded ' + self.retryCount + ' attempts to lock the resource "' + resource + '".'));
+			}
 		}
 	}
 };
@@ -181,14 +186,29 @@ Redlock.prototype.lock = function lock(resource, value, ttl, callback) {
 // ------
 // This method unlocks the provided lock from all servers still persisting it. This is a
 // best-effort attempt and as such fails silently.
-Redlock.prototype.unlock = function unlock(lock) {
+Redlock.prototype.unlock = function unlock(lock, callback) {
+
+	// the lock has expired
+	if(lock.expiration < Date.now()) {
+		if(typeof callback === 'function') callback();
+		return;
+	}
+
+	// invalidate the lock
+	lock.expiration = 0;
 
-	// TODO: invalidate the lease
+	// the number of async redis calls still waiting to finish
+	var waiting = this.servers.length;
 
 	// release the lock on each server
 	this.servers.forEach(function(server){
-		server.eval(unlockScript, 1, lock.resource, lock.value);
+		server.eval(unlockScript, 1, lock.resource, lock.value, loop);
 	});
+
+	function loop(err, response) {
+		if(waiting-- > 1) return;
+		if(typeof callback === 'function') callback();
+	}
 };
 
 
@@ -199,7 +219,7 @@ Redlock.prototype.extend = function extend(lock, ttl, callback) {
 	var self = this;
 
 	// the lock has expired
-	if(lock.expiration >= Date.now())
+	if(lock.expiration < Date.now())
 		return callback(new LockError('Cannot extend lock on resource "' + lock.resource + '" because the lock has already expired.'));
 
 	// extend the lock

test.js

@@ -1,2 +1,101 @@
 'use strict';
 
+var assert = require('chai').assert;
+var client = require('redis').createClient();
+var Redlock = require('./redlock');
+
+var redlock = new Redlock({
+	retryCount: 2,
+	retryDelay: 150
+}, client);
+
+var resource = 'Redlock:test:resource';
+
+describe('Redlock', function(){
+
+	before(function(done){
+		client.del(resource, done);
+	});
+
+	var one;
+	it('should lock a resource', function(done){
+		redlock.lock(resource, 200, function(err, lock){
+			if(err) throw err;
+			assert.isObject(lock);
+			assert.isAbove(lock.expiration, Date.now());
+			one = lock;
+			done();
+		});
+	});
+
+	var two;
+	var two_expiration;
+	it('should wait until a lock expires before issuing another lock', function(done){
+		assert(one, 'Could not run because a required previous test failed.');
+		redlock.lock(resource, 800, function(err, lock){
+			if(err) throw err;
+			assert.isObject(lock);
+			assert.isAbove(lock.expiration, Date.now());
+			assert.isAbove(Date.now(), one.expiration);
+			two = lock;
+			two_expiration = lock.expiration;
+			done();
+		});
+	});
+
+	it('should unlock a resource', function(done){
+		assert(two, 'Could not run because a required previous test failed.');
+		two.unlock(done);
+	});
+
+	var three;
+	it('should issue another lock immediately after a resource is unlocked', function(done){
+		assert(two_expiration, 'Could not run because a required previous test failed.');
+		redlock.lock(resource, 800, function(err, lock){
+			if(err) throw err;
+			assert.isObject(lock);
+			assert.isAbove(lock.expiration, Date.now());
+			assert.isBelow(Date.now(), two_expiration);
+			three = lock;
+			done();
+		});
+	});
+
+	var four;
+	it('should extend an unexpired lock', function(done){
+		assert(three, 'Could not run because a required previous test failed.');
+		three.extend(800, function(err, lock){
+			if(err) throw err;
+			assert.isObject(lock);
+			assert.isAbove(lock.expiration, Date.now());
+			assert.isAbove(lock.expiration, three.expiration);
+			four = lock;
+			done();
+		});
+	});
+
+	it('should fail after the maximum retry count is exceeded', function(done){
+		assert(four, 'Could not run because a required previous test failed.');
+		redlock.lock(resource, 200, function(err, lock){
+			assert.isNotNull(err);
+			assert.equal(err.name, 'LockError');
+			done();
+		});
+	});
+
+	it('should fail to extend an expired lock', function(done){
+		assert(four, 'Could not run because a required previous test failed.');
+		setTimeout(function(){
+			three.extend(800, function(err, lock){
+				assert.isNotNull(err);
+				assert.equal(err.name, 'LockError');
+				done();
+			});
+		}, four.expiration - Date.now() + 100)
+	});
+
+	after(function(done){
+		client.del(resource, done);
+	});
+
+});