From 14c1ca6943f735cccda4165bda95d23c8b76031d Mon Sep 17 00:00:00 2001
From: Zihua Li <i@zihua.li>
Date: Thu, 1 Oct 2015 15:37:22 +0800
Subject: [PATCH 1/3] Add support for ioredis

---
 index.js      |   2 +-
 package.json  |   5 +-
 test/index.js | 325 +++++++++++++++++++++++++-------------------------
 3 files changed, 167 insertions(+), 165 deletions(-)

diff --git a/index.js b/index.js
index 52ce330..0a8889c 100644
--- a/index.js
+++ b/index.js
@@ -76,7 +76,7 @@ Limiter.prototype.get = function (fn) {
         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 (!res || !res[0] || !res[0][1]) return mget();
 
         fn(null, {
           total: max,
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);
+            });
+          }
+        });
       });
     });
   });

From 41df16210638fcdf3d953bdee1ec7c35c468b0b4 Mon Sep 17 00:00:00 2001
From: luin <i@zihua.li>
Date: Sun, 11 Oct 2015 16:52:07 +0800
Subject: [PATCH 2/3] Refactor conditions for readability

---
 index.js | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/index.js b/index.js
index 0a8889c..8eba51a 100644
--- a/index.js
+++ b/index.js
@@ -74,9 +74,17 @@ 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] || !res[0][1]) return mget();
+
+        // If the request has failed, it means the values already
+        // exist in which case we need to get the latest values.
+        if (!res[0]) return mget();
+        if (Array.isArray(res[0])) {
+          // ioredis
+          if (!res[0][1]) return mget();
+        } else {
+          // node_redis
+          if (!res[0]) return mget();
+        }
 
         fn(null, {
           total: max,

From e46872ac9e5e03fbb81a53e40247d02495bc55a4 Mon Sep 17 00:00:00 2001
From: luin <i@zihua.li>
Date: Mon, 12 Oct 2015 15:33:43 +0800
Subject: [PATCH 3/3] Wrap the detections in a function

---
 index.js | 32 +++++++++++++++++++++++---------
 1 file changed, 23 insertions(+), 9 deletions(-)

diff --git a/index.js b/index.js
index 8eba51a..16df973 100644
--- a/index.js
+++ b/index.js
@@ -77,14 +77,7 @@ Limiter.prototype.get = function (fn) {
 
         // If the request has failed, it means the values already
         // exist in which case we need to get the latest values.
-        if (!res[0]) return mget();
-        if (Array.isArray(res[0])) {
-          // ioredis
-          if (!res[0][1]) return mget();
-        } else {
-          // node_redis
-          if (!res[0]) return mget();
-        }
+        if (isFirstReplyNull(res)) return mget();
 
         fn(null, {
           total: max,
@@ -113,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();
       });
@@ -133,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];
+}