From 6171578c53afefb26d491d601ede236e94b09622 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Greg=20Berg=C3=A9?= Date: Thu, 29 Jan 2015 15:48:00 +0100 Subject: [PATCH] Refactor code and also fix issue with "unlock" event. Fix #6 --- lib/locky.js | 49 ++++++++++++++++++++++++++++++++++--------------- package.json | 1 + test/locky.js | 32 ++++++++++++++++++++++++-------- 3 files changed, 59 insertions(+), 23 deletions(-) diff --git a/lib/locky.js b/lib/locky.js index 693439c..771485b 100644 --- a/lib/locky.js +++ b/lib/locky.js @@ -5,9 +5,10 @@ var events = require('events'); var util = require('util'); var _ = require('lodash'); -var resourceKey = require('./resource-key'); var redis = require('then-redis'); var Promise = require('bluebird'); +var pipeEvent = require('pipe-event'); +var resourceKey = require('./resource-key'); /** * Expose module. @@ -34,9 +35,9 @@ function Locky(options) { locky.ttl = options.ttl; locky.redis = locky._createRedisClient(options.redis); - this.redis.on('error', function (err) { - console.log(err); - }); + pipeEvent(['error'], locky.redis, this); + + locky._resourceTimeouts = {}; events.EventEmitter.call(locky); } @@ -57,7 +58,7 @@ util.inherits(Locky, events.EventEmitter); * @param {Promise} [callback] Optional callback */ -Locky.prototype.lock = function lock(options, callback) { +Locky.prototype.lock = function (options, callback) { var locky = this; options = options || {}; @@ -68,7 +69,7 @@ Locky.prototype.lock = function lock(options, callback) { // Define the method to use. var method = options.force ? 'set' : 'setnx'; - // Set the lock key. + // Set the lock key. return locky.redis[method](key, options.locker) .then(function (res) { var success = res === 1 || res === 'OK'; @@ -96,7 +97,7 @@ Locky.prototype.lock = function lock(options, callback) { * @param {function} [callback] Optional callback */ -Locky.prototype.refresh = function refresh(resource, callback) { +Locky.prototype.refresh = function (resource, callback) { var locky = this; return new Promise(function (resolve, reject) { @@ -122,15 +123,19 @@ Locky.prototype.refresh = function refresh(resource, callback) { * @param {function} [callback] Optional callback */ -Locky.prototype.unlock = function unlock(resource, callback) { +Locky.prototype.unlock = function (resource, callback) { var locky = this; // Format key with resource id. var key = resourceKey.format(resource); // Remove the key. - return locky.redis.del(key).then(function (res) { - if (res !== 0) locky.emit('unlock', resource); + return locky.redis.del(key) + .then(function (res) { + if (res === 0) return; + + locky.emit('unlock', resource); + locky._clearExpiration(resource); }) .nodeify(callback); }; @@ -142,7 +147,7 @@ Locky.prototype.unlock = function unlock(resource, callback) { * @param {function} [callback] Optional callback */ -Locky.prototype.getLocker = function getLocker(resource, callback) { +Locky.prototype.getLocker = function (resource, callback) { return this.redis.get(resourceKey.format(resource)).nodeify(callback); }; @@ -152,7 +157,7 @@ Locky.prototype.getLocker = function getLocker(resource, callback) { * @param {function} [callback] Optional callback */ -Locky.prototype.close = function close(callback) { +Locky.prototype.close = function (callback) { return this.redis.quit().nodeify(callback); }; @@ -162,8 +167,22 @@ Locky.prototype.close = function close(callback) { * @param {string} resource Resource */ -Locky.prototype._listenExpiration = function _listenExpiration(resource) { - setTimeout(this._onExpire.bind(this, resource), this.ttl); +Locky.prototype._listenExpiration = function (resource) { + var bindedExpire = this._onExpire.bind(this, resource); + this._resourceTimeouts[resource] = setTimeout(bindedExpire, this.ttl); +}; + +/** + * Clear timeout on expiration. + * + * @param {string} resource Resource + */ + +Locky.prototype._clearExpiration = function (resource) { + var timeout = this._resourceTimeouts[resource]; + + if (timeout) + clearTimeout(timeout); }; /** @@ -172,7 +191,7 @@ Locky.prototype._listenExpiration = function _listenExpiration(resource) { * @param {string} resource */ -Locky.prototype._onExpire = function _onExpire(resource) { +Locky.prototype._onExpire = function (resource) { var locky = this; return locky.getLocker(resource).then(function (locker) { diff --git a/package.json b/package.json index 07091d3..8747e82 100644 --- a/package.json +++ b/package.json @@ -29,6 +29,7 @@ "dependencies": { "bluebird": "^2.9.3", "lodash": "^3.0.0", + "pipe-event": "^0.1.0", "redis": "^0.12.1", "then-redis": "^1.3.0" } diff --git a/test/locky.js b/test/locky.js index 1310227..b4e43ab 100644 --- a/test/locky.js +++ b/test/locky.js @@ -130,10 +130,8 @@ describe('Locky', function () { }); it('should emit an expire event when the lock expire', function () { - this.timeout(3000); - var spy = sinon.spy(); - locky = createLocky({ttl: 1000}); + locky = createLocky({ttl: 100}); locky.on('expire', spy); return locky.lock({ @@ -141,7 +139,7 @@ describe('Locky', function () { locker: 'john' }) .then(function () { - return Promise.delay(2100); + return Promise.delay(200); }) .then(function () { expect(spy).to.be.calledWith('article4'); @@ -182,10 +180,8 @@ describe('Locky', function () { }); it('should emit an expire event when the lock expire', function () { - this.timeout(3000); - var spy = sinon.spy(); - locky = createLocky({ttl: 1000}); + locky = createLocky({ttl: 100}); locky.on('expire', spy); locky.redis.multi(); @@ -194,7 +190,7 @@ describe('Locky', function () { return locky.redis.exec() .then(function () { locky.refresh('article8'); - return Promise.delay(2100); + return Promise.delay(200); }) .then(function () { expect(spy).to.be.calledWith('article8'); @@ -241,6 +237,26 @@ describe('Locky', function () { expect(spy).to.not.be.called; }); }); + + it('should not expire if we "unlock"', function () { + var spy = sinon.spy(); + locky = createLocky({ttl: 100}); + locky.on('expire', spy); + + return locky.lock({ + resource: 'article13', + locker: 'john' + }) + .then(function () { + return locky.unlock('article13'); + }) + .then(function () { + return Promise.delay(200); + }) + .then(function () { + expect(spy).to.not.be.called; + }); + }); }); describe('#getLocker', function () {