fixes #29

History.md

@@ -11,4 +11,4 @@
 
 ## v1.0.1 / 2014-03-14
 
-* fix race condition resetting the keys
+* fix race condition resetting the keys.

index.js

@@ -36,11 +36,11 @@ function Limiter(opts) {
  * @api public
  */
 
-Limiter.prototype.inspect = function () {
-  return '<Limiter id='
-    + this.id + ', duration='
-    + this.duration + ', max='
-    + this.max + '>';
+Limiter.prototype.inspect = function() {
+  return '<Limiter id=' +
+    this.id + ', duration=' +
+    this.duration + ', max=' +
+    this.max + '>';
 };
 
 /**
@@ -57,7 +57,7 @@ Limiter.prototype.inspect = function () {
  * @api public
  */
 
-Limiter.prototype.get = function (fn) {
+Limiter.prototype.get = function(fn) {
   var count = this.prefix + 'count';
   var limit = this.prefix + 'limit';
   var reset = this.prefix + 'reset';
@@ -72,7 +72,7 @@ Limiter.prototype.get = function (fn) {
       .set([count, max, 'PX', duration, 'NX'])
       .set([limit, max, 'PX', duration, 'NX'])
       .set([reset, ex, 'PX', duration, 'NX'])
-      .exec(function (err, res) {
+      .exec(function(err, res) {
         if (err) return fn(err);
 
         // If the request has failed, it means the values already
@@ -104,21 +104,22 @@ Limiter.prototype.get = function (fn) {
     }
 
     db.multi()
-      .set([count, n - 1, 'PX', ex * 1000 - dateNow, 'XX'])
+      .decr(count)
+      .pexpire([count, ex * 1000 - dateNow])
       .pexpire([limit, ex * 1000 - dateNow])
       .pexpire([reset, ex * 1000 - dateNow])
-      .exec(function (err, res) {
+      .exec(function(err, res) {
         if (err) return fn(err);
         if (isFirstReplyNull(res)) return mget();
-        n = n - 1;
+        n = Array.isArray(res[0]) ? ~~res[0][1] : ~~res[0];
         done();
       });
   }
 
   function mget() {
-    db.watch([count], function (err) {
+    db.watch([count], function(err) {
       if (err) return fn(err);
-      db.mget([count, limit, reset], function (err, res) {
+      db.mget([count, limit, reset], function(err, res) {
         if (err) return fn(err);
         if (!res[0] && res[0] !== 0) return create();
 

package.json

@@ -9,8 +9,12 @@
     "limiter",
     "limit"
   ],
+  "scripts": {
+    "test": "mocha"
+  },
   "dependencies": {},
   "devDependencies": {
+    "async": "2.1.4",
     "ioredis": "1.15.1",
     "mocha": "*",
     "redis": "2.6.0-1",

test/index.js

@@ -1,11 +1,12 @@
 require('should');
-var Limiter = require('..');
+var Limiter = require('..'),
+  async = require('async');
 
 // Uncomment the following line if you want to see
 // debug logs from the node-redis module.
 //redis.debug_mode = true;
 
-['redis', 'ioredis'].forEach(function (redisModuleName) {
+['redis', 'ioredis'].forEach(function(redisModuleName) {
   var redisModule = require(redisModuleName);
   var db = require(redisModuleName).createClient();
   describe('Limiter with ' + redisModuleName, function() {
@@ -146,7 +147,7 @@ var Limiter = require('..');
             .pttl(['limit:something:count'])
             .pttl(['limit:something:limit'])
             .pttl(['limit:something:reset'])
-            .exec(function (err, res) {
+            .exec(function(err, res) {
               if (err) return done(err);
               var ttlCount = (typeof res[0] === 'number') ? res[0] : res[0][1];
               var ttlLimit = (typeof res[1] === 'number') ? res[1] : res[1][1];
@@ -211,7 +212,8 @@ var Limiter = require('..');
 
             if (err) {
               done(err);
-            } else {
+            }
+            else {
               responses.forEach(function(res) {
                 res[1].remaining.should.equal(left < 0 ? 0 : left);
                 left--;
@@ -230,7 +232,8 @@ var Limiter = require('..');
         limits[0].get(function(err, res) {
           if (err) {
             done(err);
-          } else {
+          }
+          else {
             res.remaining.should.equal(left--);
 
             // Simulate multiple concurrent requests.
@@ -241,5 +244,31 @@ var Limiter = require('..');
         });
       });
     });
+
+    describe('when limiter is called in parallel by multiple clients', function() {
+      var max = 6,
+        limiter;
+
+      limiter = new Limiter({
+        duration: 10000,
+        max: max,
+        id: 'asyncsomething',
+        db: redisModule.createClient()
+      });
+
+      it('should set the count properly without race conditions', function(done) {
+        async.times(max, function(n, next) {
+            limiter.get(next);
+          },
+          function(errs, limits) {
+
+            limits.forEach(function(limit) {
+              limit.remaining.should.equal(max--);
+            });
+            done();
+
+          });
+      });
+    })
   });
 });