diff --git a/index.js b/index.js index 52ce330..16df973 100644 --- a/index.js +++ b/index.js @@ -74,9 +74,10 @@ Limiter.prototype.get = function (fn) { .set([reset, ex, 'PX', duration, 'NX']) .exec(function (err, res) { if (err) return fn(err); - // If the request has failed, it means the values already - // exist in which case we need to get the latest values. - if (!res || !res[0]) return mget(); + + // If the request has failed, it means the values already + // exist in which case we need to get the latest values. + if (isFirstReplyNull(res)) return mget(); fn(null, { total: max, @@ -105,7 +106,7 @@ Limiter.prototype.get = function (fn) { .set([count, n - 1, 'PX', ex * 1000 - Date.now(), 'XX']) .exec(function (err, res) { if (err) return fn(err); - if (!res || !res[0]) return mget(); + if (isFirstReplyNull(res)) return mget(); n = n - 1; done(); }); @@ -125,3 +126,24 @@ Limiter.prototype.get = function (fn) { mget(); }; + +/** + * Check whether the first item of multi replies is null, + * works with ioredis and node_redis + * + * @param {Array} replies + * @return {Boolean} + * @api private + */ + +function isFirstReplyNull(replies) { + if (!replies) { + return true; + } + + return Array.isArray(replies[0]) ? + // ioredis + !replies[0][1] : + // node_redis + !replies[0]; +} diff --git a/package.json b/package.json index 62987f0..4cce56e 100644 --- a/package.json +++ b/package.json @@ -11,9 +11,10 @@ ], "dependencies": {}, "devDependencies": { + "ioredis": "^1.9.0", "mocha": "*", - "should": "*", - "redis": "0.10.2" + "redis": "0.10.2", + "should": "*" }, "license": "MIT", "contributors": [ diff --git a/test/index.js b/test/index.js index 4dde103..acd5b46 100644 --- a/test/index.js +++ b/test/index.js @@ -1,219 +1,220 @@ require('should'); var Limiter = require('..'); -var redis = require('redis'); // Uncomment the following line if you want to see // debug logs from the node-redis module. //redis.debug_mode = true; -var db = redis.createClient(); - -describe('Limiter', function() { - beforeEach(function(done) { - db.keys('limit:*', function(err, keys) { - if (err) return done(err); - if (!keys.length) return done(); - var args = keys.concat(done); - db.del.apply(db, args); +['redis', 'ioredis'].forEach(function (redisModuleName) { + var redisModule = require(redisModuleName); + var db = require(redisModuleName).createClient(); + describe('Limiter with ' + redisModuleName, function() { + beforeEach(function(done) { + db.keys('limit:*', function(err, keys) { + if (err) return done(err); + if (!keys.length) return done(); + var args = keys.concat(done); + db.del.apply(db, args); + }); }); - }); - describe('.total', function() { - it('should represent the total limit per reset period', function(done) { - var limit = new Limiter({ - max: 5, - id: 'something', - db: db - }); - limit.get(function(err, res) { - res.total.should.equal(5); - done(); + describe('.total', function() { + it('should represent the total limit per reset period', function(done) { + var limit = new Limiter({ + max: 5, + id: 'something', + db: db + }); + limit.get(function(err, res) { + res.total.should.equal(5); + done(); + }); }); }); - }); - describe('.remaining', function() { - it('should represent the number of requests remaining in the reset period', function(done) { - var limit = new Limiter({ - max: 5, - duration: 100000, - id: 'something', - db: db - }); - limit.get(function(err, res) { - res.remaining.should.equal(5); + describe('.remaining', function() { + it('should represent the number of requests remaining in the reset period', function(done) { + var limit = new Limiter({ + max: 5, + duration: 100000, + id: 'something', + db: db + }); limit.get(function(err, res) { - res.remaining.should.equal(4); + res.remaining.should.equal(5); limit.get(function(err, res) { - res.remaining.should.equal(3); - done(); + res.remaining.should.equal(4); + limit.get(function(err, res) { + res.remaining.should.equal(3); + done(); + }); }); }); }); }); - }); - describe('.reset', function() { - it('should represent the next reset time in UTC epoch seconds', function(done) { - var limit = new Limiter({ - max: 5, - duration: 60000, - id: 'something', - db: db - }); - limit.get(function(err, res) { - var left = res.reset - (Date.now() / 1000); - left.should.be.below(60); - done(); + describe('.reset', function() { + it('should represent the next reset time in UTC epoch seconds', function(done) { + var limit = new Limiter({ + max: 5, + duration: 60000, + id: 'something', + db: db + }); + limit.get(function(err, res) { + var left = res.reset - (Date.now() / 1000); + left.should.be.below(60); + done(); + }); }); }); - }); - describe('when the limit is exceeded', function() { - it('should retain .remaining at 0', function(done) { - var limit = new Limiter({ - max: 2, - id: 'something', - db: db - }); - limit.get(function(err, res) { - res.remaining.should.equal(2); + describe('when the limit is exceeded', function() { + it('should retain .remaining at 0', function(done) { + var limit = new Limiter({ + max: 2, + id: 'something', + db: db + }); limit.get(function(err, res) { - res.remaining.should.equal(1); + res.remaining.should.equal(2); limit.get(function(err, res) { - // function caller should reject this call - res.remaining.should.equal(0); - done(); + res.remaining.should.equal(1); + limit.get(function(err, res) { + // function caller should reject this call + res.remaining.should.equal(0); + done(); + }); }); }); }); }); - }); - describe('when the duration is exceeded', function() { - it('should reset', function(done) { - this.timeout(5000); - var limit = new Limiter({ - duration: 2000, - max: 2, - id: 'something', - db: db - }); - limit.get(function(err, res) { - res.remaining.should.equal(2); + describe('when the duration is exceeded', function() { + it('should reset', function(done) { + this.timeout(5000); + var limit = new Limiter({ + duration: 2000, + max: 2, + id: 'something', + db: db + }); limit.get(function(err, res) { - res.remaining.should.equal(1); - setTimeout(function() { - limit.get(function(err, res) { - var left = res.reset - (Date.now() / 1000); - left.should.be.below(2); - res.remaining.should.equal(2); - done(); - }); - }, 3000); + res.remaining.should.equal(2); + limit.get(function(err, res) { + res.remaining.should.equal(1); + setTimeout(function() { + limit.get(function(err, res) { + var left = res.reset - (Date.now() / 1000); + left.should.be.below(2); + res.remaining.should.equal(2); + done(); + }); + }, 3000); + }); }); }); }); - }); - describe('when multiple successive calls are made', function() { - it('the next calls should not create again the limiter in Redis', function(done) { - var limit = new Limiter({ - duration: 10000, - max: 2, - id: 'something', - db: db - }); - limit.get(function(err, res) { - res.remaining.should.equal(2); - }); + describe('when multiple successive calls are made', function() { + it('the next calls should not create again the limiter in Redis', function(done) { + var limit = new Limiter({ + duration: 10000, + max: 2, + id: 'something', + db: db + }); + limit.get(function(err, res) { + res.remaining.should.equal(2); + }); - limit.get(function(err, res) { - res.remaining.should.equal(1); - done(); + limit.get(function(err, res) { + res.remaining.should.equal(1); + done(); + }); }); }); - }); - describe('when trying to decrease before setting value', function() { - it('should create with ttl when trying to decrease', function(done) { - var limit = new Limiter({ - duration: 10000, - max: 2, - id: 'something', - db: db - }); - db.setex('limit:something:count', -1, 1, function() { - limit.get(function(err, res) { - res.remaining.should.equal(2); + describe('when trying to decrease before setting value', function() { + it('should create with ttl when trying to decrease', function(done) { + var limit = new Limiter({ + duration: 10000, + max: 2, + id: 'something', + db: db + }); + db.setex('limit:something:count', -1, 1, function() { limit.get(function(err, res) { - res.remaining.should.equal(1); + res.remaining.should.equal(2); limit.get(function(err, res) { - res.remaining.should.equal(0); - done(); + res.remaining.should.equal(1); + limit.get(function(err, res) { + res.remaining.should.equal(0); + done(); + }); }); }); }); }); }); - }); - describe('when multiple concurrent clients modify the limit', function() { - var clientsCount = 7, - max = 5, - left = max, - limits = []; - - for (var i = 0; i < clientsCount; ++i) { - limits.push(new Limiter({ - duration: 10000, - max: max, - id: 'something', - db: redis.createClient() - })); - } - - it('should prevent race condition and properly set the expected value', function(done) { - var responses = []; - - function complete() { - responses.push(arguments); - - if (responses.length == clientsCount) { - // If there were any errors, report. - var err = responses.some(function(res) { - return res[0]; - }); + describe('when multiple concurrent clients modify the limit', function() { + var clientsCount = 7, + max = 5, + left = max, + limits = []; + + for (var i = 0; i < clientsCount; ++i) { + limits.push(new Limiter({ + duration: 10000, + max: max, + id: 'something', + db: redisModule.createClient() + })); + } - if (err) { - done(err); - } else { - responses.forEach(function(res) { - res[1].remaining.should.equal(left < 0 ? 0 : left); - left--; + it('should prevent race condition and properly set the expected value', function(done) { + var responses = []; + + function complete() { + responses.push(arguments); + + if (responses.length == clientsCount) { + // If there were any errors, report. + var err = responses.some(function(res) { + return res[0]; }); - for (var i = max - 1; i < clientsCount; ++i) { - responses[i][1].remaining.should.equal(0); - } + if (err) { + done(err); + } else { + responses.forEach(function(res) { + res[1].remaining.should.equal(left < 0 ? 0 : left); + left--; + }); + + for (var i = max - 1; i < clientsCount; ++i) { + responses[i][1].remaining.should.equal(0); + } - done(); + done(); + } } } - } - // Warm up and prepare the data. - limits[0].get(function(err, res) { - if (err) { - done(err); - } else { - res.remaining.should.equal(left--); + // Warm up and prepare the data. + limits[0].get(function(err, res) { + if (err) { + done(err); + } else { + res.remaining.should.equal(left--); - // Simulate multiple concurrent requests. - limits.forEach(function(limit) { - limit.get(complete); - }); - } + // Simulate multiple concurrent requests. + limits.forEach(function(limit) { + limit.get(complete); + }); + } + }); }); }); });