Merge pull request #3 from NodeRedis/v.1.2

BridgeAR · BridgeAR · commit 004f76d78245 · 2016-04-14T10:42:54.000+02:00
v.1.2
diff --git a/.gitignore b/.gitignore
@@ -6,6 +6,7 @@ logs
 pids
 *.pid
 *.seed
+*.rdb
 
 # Directory for instrumented libs generated by jscoverage/JSCover
 lib-cov
diff --git a/.travis.yml b/.travis.yml
@@ -5,3 +5,5 @@ node_js:
   - "0.12"
   - "4"
   - "5"
+after_success:
+  - CODECLIMATE_REPO_TOKEN=b57723fafcf0516f275d6b380cd506fd082ea88d86507eb82c8abd489b9b9a09 node ./node_modules/.bin/codeclimate-test-reporter < coverage/lcov.info
diff --git a/README.md b/README.md
@@ -1,6 +1,8 @@
 # Redis Commands
 
 [![Build Status](https://travis-ci.org/NodeRedis/redis-commands.png?branch=master)](https://travis-ci.org/NodeRedis/redis-commands)
+[![Code Climate](https://codeclimate.com/github/NodeRedis/redis-commands/badges/gpa.svg)](https://codeclimate.com/github/NodeRedis/redis-commands)
+[![Test Coverage](https://codeclimate.com/github/NodeRedis/redis-commands/badges/coverage.svg)](https://codeclimate.com/github/NodeRedis/redis-commands/coverage)
 
 This module exports all the commands that Redis supports.
 
diff --git a/changelog.md b/changelog.md
@@ -1,3 +1,17 @@
+## v.1.2.0 - xx Apr, 2016
+
+Features
+
+-  Added support for `MIGRATE [...] KEYS key1, key2` (Redis >= v.3.0.6)
+-  Added build sanity check for unhandled commands with moveable keys
+-  Rebuild the commands with the newest unstable release
+-  Improved performance of .getKeyIndexes()
+
+Bugfix
+
+-  Fixed command command returning the wrong arity due to a Redis bug
+-  Fixed brpop command returning the wrong keystop due to a Redis bug
+
 ## v.1.1.0 - 09 Feb, 2016
 
 Features
diff --git a/commands.json b/commands.json
@@ -60,6 +60,16 @@
     "keyStop": 1,
     "step": 1
   },
+  "bitfield": {
+    "arity": -2,
+    "flags": [
+      "write",
+      "denyoom"
+    ],
+    "keyStart": 1,
+    "keyStop": 1,
+    "step": 1
+  },
   "bitop": {
     "arity": -4,
     "flags": [
@@ -96,7 +106,7 @@
       "noscript"
     ],
     "keyStart": 1,
-    "keyStop": 1,
+    "keyStop": -2,
     "step": 1
   },
   "brpoplpush": {
@@ -131,7 +141,7 @@
     "step": 0
   },
   "command": {
-    "arity": 0,
+    "arity": 1,
     "flags": [
       "readonly",
       "loading",
@@ -351,7 +361,7 @@
   "georadius": {
     "arity": -6,
     "flags": [
-      "readonly"
+      "write"
     ],
     "keyStart": 1,
     "keyStop": 1,
@@ -360,7 +370,7 @@
   "georadiusbymember": {
     "arity": -5,
     "flags": [
-      "readonly"
+      "write"
     ],
     "keyStart": 1,
     "keyStop": 1,
@@ -745,11 +755,12 @@
   "migrate": {
     "arity": -6,
     "flags": [
-      "write"
+      "write",
+      "movablekeys"
     ],
-    "keyStart": 3,
-    "keyStop": 3,
-    "step": 1
+    "keyStart": 0,
+    "keyStop": 0,
+    "step": 0
   },
   "monitor": {
     "arity": 1,
diff --git a/index.js b/index.js
@@ -73,11 +73,12 @@ exports.getKeyIndexes = function (commandName, args, options) {
     throw new Error('Expect args to be an array');
   }
 
-  var parseExternalKey = options && options.parseExternalKey;
-
   var keys = [];
-  var i, range, keyStart, keyStop;
+  var i, keyStart, keyStop, parseExternalKey;
   switch (commandName) {
+  case 'zunionstore':
+  case 'zinterstore':
+    keys.push(0);
   case 'eval':
   case 'evalsha':
     keyStop = Number(args[1]) + 2;
@@ -86,6 +87,7 @@ exports.getKeyIndexes = function (commandName, args, options) {
     }
     break;
   case 'sort':
+    parseExternalKey = options && options.parseExternalKey;
     keys.push(0);
     for (i = 1; i < args.length - 1; i++) {
       if (typeof args[i] !== 'string') {
@@ -114,18 +116,25 @@ exports.getKeyIndexes = function (commandName, args, options) {
       }
     }
     break;
-  case 'zunionstore':
-  case 'zinterstore':
-    keys.push(0);
-    keyStop = Number(args[1]) + 2;
-    for (i = 2; i < keyStop; i++) {
-      keys.push(i);
+  case 'migrate':
+    if (args[2] === '') {
+      for (i = 5; i < args.length - 1; i++) {
+        if (args[i].toUpperCase() === 'KEYS') {
+          for (var j = i + 1; j < args.length; j++) {
+            keys.push(j);
+          }
+          break;
+        }
+      }
+    } else {
+      keys.push(2);
     }
     break;
   default:
-    keyStart = command.keyStart - 1;
-    keyStop = command.keyStop > 0 ? command.keyStop : args.length + command.keyStop + 1;
-    if (keyStart >= 0 && keyStop <= args.length && keyStop > keyStart && command.step > 0) {
+    // step has to be at least one in this case, otherwise the command does not contain a key
+    if (command.step > 0) {
+      keyStart = command.keyStart - 1;
+      keyStop = command.keyStop > 0 ? command.keyStop : args.length + command.keyStop + 1;
       for (i = keyStart; i < keyStop; i += command.step) {
         keys.push(i);
       }
diff --git a/package.json b/package.json
@@ -5,6 +5,9 @@
   "main": "index.js",
   "scripts": {
     "test": "mocha",
+    "posttest": "npm run coverage && npm run coverage:check",
+    "coverage": "node ./node_modules/istanbul/lib/cli.js cover --preserve-comments ./node_modules/mocha/bin/_mocha -- -R spec",
+    "coverage:check": "node ./node_modules/istanbul/lib/cli.js check-coverage --branch 100 --statement 100",
     "build": "node tools/build"
   },
   "repository": {
@@ -24,7 +27,9 @@
   "homepage": "https://github.com/NodeRedis/redis-commonds",
   "devDependencies": {
     "chai": "^3.4.0",
-    "ioredis": "^1.0.8",
+    "codeclimate-test-reporter": "^0.3.1",
+    "ioredis": "^2.0.0-rc2",
+    "istanbul": "^0.4.3",
     "json-stable-stringify": "^1.0.0",
     "mocha": "^2.2.1"
   }
diff --git a/test/index.js b/test/index.js
@@ -55,13 +55,30 @@ describe('redis-commands', function () {
       expect(commands.hasFlag('select', 'denyoom')).to.eql(false);
       expect(commands.hasFlag('quit', 'denyoom')).to.eql(false);
     });
+
+    it('should throw on unknown commands', function () {
+      expect(function () { commands.hasFlag('UNKNOWN'); }).to.throw(Error);
+    });
   });
 
   describe('.getKeyIndexes()', function () {
     var index = commands.getKeyIndexes;
 
+    it('should throw on unknown commands', function () {
+      expect(function () { index('UNKNOWN'); }).to.throw(Error);
+    });
+
+    it('should throw on faulty args', function () {
+      expect(function () { index('get', 'foo'); }).to.throw(Error);
+    });
+
+    it('should return an empty array if no keys exist', function () {
+      expect(index('auth', [])).to.eql([]);
+    });
+
     it('should return key indexes', function () {
       expect(index('set', ['foo', 'bar'])).to.eql([0]);
+      expect(index('del', ['foo'])).to.eql([0]);
       expect(index('get', ['foo'])).to.eql([0]);
       expect(index('mget', ['foo', 'bar'])).to.eql([0, 1]);
       expect(index('mset', ['foo', 'v1', 'bar', 'v2'])).to.eql([0, 2]);
@@ -70,6 +87,9 @@ describe('redis-commands', function () {
       expect(index('evalsha', ['23123', '2', 'foo', 'bar', 'zoo'])).to.eql([2, 3]);
       expect(index('sort', ['key'])).to.eql([0]);
       expect(index('zunionstore', ['out', '2', 'zset1', 'zset2', 'WEIGHTS', '2', '3'])).to.eql([0, 2, 3]);
+      expect(index('migrate', ['127.0.0.1', 6379, 'foo', 0, 0, 'COPY'])).to.eql([2]);
+      expect(index('migrate', ['127.0.0.1', 6379, '', 0, 0, 'REPLACE', 'KEYS', 'foo', 'bar'])).to.eql([7, 8]);
+      expect(index('migrate', ['127.0.0.1', 6379, '', 0, 0, 'KEYS', 'foo', 'bar'])).to.eql([6, 7]);
     });
 
     it('should support numeric argument', function () {
@@ -89,7 +109,7 @@ describe('redis-commands', function () {
         expect(index('sort', ['key', 'BY', 'hash:*->field'], {
           parseExternalKey: true
         })).to.eql([0, [2, 6]]);
-        expect(index('sort', ['key', 'BY', 'hash:*->field', 'LIMIT', 2, 3, 'GET', 'gk', 'GET', '#', 'Get', 'gh->f*', 'DESC', 'ALPHA', 'STORE', 'store'], {
+        expect(index('sort', ['key', 'BY', 'hash:*->field', 'LIMIT', 2, 3, 'GET', new Buffer('gk'), 'GET', '#', 'Get', 'gh->f*', 'DESC', 'ALPHA', 'STORE', 'store'], {
           parseExternalKey: true
         })).to.eql([0, [2, 6], [7, 2], [11, 2], 15]);
       });
diff --git a/tools/build.js b/tools/build.js
@@ -2,20 +2,30 @@ var fs = require('fs');
 var path = require('path');
 var stringify = require('json-stable-stringify');
 var commandPath = path.join(__dirname, '..', 'commands.json');
+var redisCommands = require('../');
 
 var Redis = require('ioredis');
 var redis = new Redis(process.env.REDIS_URI);
 
-redis.command(function (err, res) {
+redis.command().then(function (res) {
   redis.disconnect();
 
-  if (err) {
-    throw err;
-  }
+  // Find all special handled cases
+  var movableKeys = String(redisCommands.getKeyIndexes).match(/case '[a-z-]+':/g).map(function (entry) {
+    return entry.replace(/^case \'|\':$/g, '');
+  });
 
   var commands = res.reduce(function (prev, current) {
+    var currentCommandPos = movableKeys.indexOf(current[0]);
+    if (currentCommandPos !== -1 && current[2].indexOf('movablekeys') !== -1) {
+      movableKeys.splice(currentCommandPos, 1);
+    }
+    // https://github.com/antirez/redis/issues/2598
+    if (current[0] === 'brpop' && current[4] === 1) {
+      current[4] = -2;
+    }
     prev[current[0]] = {
-      arity: current[1],
+      arity: current[1] || 1, // https://github.com/antirez/redis/pull/2986
       flags: current[2],
       keyStart: current[3],
       keyStop: current[4],
@@ -24,7 +34,8 @@ redis.command(function (err, res) {
     return prev;
   }, {});
 
-  // Future proof. Redis might implement this soon
+  // Future proof. Redis might implement this at some point
+  // https://github.com/antirez/redis/pull/2982
   if (!commands.quit) {
     commands.quit = {
       arity: 1,
@@ -39,6 +50,10 @@ redis.command(function (err, res) {
     }
   }
 
+  if (movableKeys.length !== 0) {
+    throw new Error('Not all commands (\'' + movableKeys.join('\', \'') + '\') with the "movablekeys" flag are handled in the code');
+  }
+
   // Use json-stable-stringify instead fo JSON.stringify
   // for easier diffing
   var content = stringify(commands, { space: '  ' });

-Original file line number
+Diff line change
 pids
 *.pid
 *.seed
 +*.rdb
 # Directory for instrumented libs generated by jscoverage/JSCover
 lib-cov