From 2ca42417bfd1b0221e98289459bf6799ad6913f4 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Fri, 2 Oct 2015 20:20:56 +0200 Subject: [PATCH] Fix explicitly passing undefined as callback --- changelog.md | 1 + index.js | 20 +- .../{geoadd.spec.js.future => geoadd.spec.js} | 3 +- test/commands/set.spec.js | 10 + test/commands/zadd.spec.js | 6 +- test/detect_buffers.spec.js | 209 ++++++++++++++++++ test/node_redis.spec.js | 209 +----------------- 7 files changed, 240 insertions(+), 218 deletions(-) rename test/commands/{geoadd.spec.js.future => geoadd.spec.js} (89%) create mode 100644 test/detect_buffers.spec.js diff --git a/changelog.md b/changelog.md index 3a34697..ab01472 100644 --- a/changelog.md +++ b/changelog.md @@ -14,6 +14,7 @@ Bugfixes: - Fix multi.hmset key not being type converted if used with an object and key not being a string (@BridgeAR) - Fix parser errors not being catched properly (@BridgeAR) - Fix a crash that could occur if a redis server does return the info command as usual #541 (@BridgeAR) +- Explicitly passing undefined as a callback statement will work again. E.g. client.publish('channel', 'message', undefined); (@BridgeAR) ## v2.0.1 - Sep 24, 2015 diff --git a/index.js b/index.js index 80e1c23..4458b68 100644 --- a/index.js +++ b/index.js @@ -629,6 +629,7 @@ RedisClient.prototype.return_reply = function (reply) { return elem.replace(/\\"/g, '"'); }); this.emit("monitor", timestamp, args); + /* istanbul ignore else: this is a safety check that we should not be able to trigger */ } else { var err = new Error("node_redis command queue state error. If you can reproduce this, please report it."); err.command_obj = command_obj; @@ -639,21 +640,14 @@ RedisClient.prototype.return_reply = function (reply) { RedisClient.prototype.send_command = function (command, args, callback) { var arg, command_obj, i, elem_count, buffer_args, stream = this.stream, command_str = "", buffered_writes = 0, err; - // if (typeof callback === "function") {} - // probably the fastest way: - // client.command([arg1, arg2], cb); (straight passthrough) - // send_command(command, [arg1, arg2], cb); if (args === undefined) { args = []; - } else if (!callback && typeof args[args.length - 1] === "function") { - // most people find this variable argument length form more convenient, but it uses arguments, which is slower - // client.command(arg1, arg2, cb); (wraps up arguments into an array) - // send_command(command, [arg1, arg2, cb]); - // client.command(arg1, arg2); (callback is optional) - // send_command(command, [arg1, arg2]); - // client.command(arg1, arg2, undefined); (callback is undefined) - // send_command(command, [arg1, arg2, undefined]); - callback = args.pop(); + } else if (!callback) { + if (typeof args[args.length - 1] === "function") { + callback = args.pop(); + } else if (typeof args[args.length - 1] === "undefined") { + args.pop(); + } } if (process.domain && callback) { diff --git a/test/commands/geoadd.spec.js.future b/test/commands/geoadd.spec.js similarity index 89% rename from test/commands/geoadd.spec.js.future rename to test/commands/geoadd.spec.js index 710b528..c989924 100644 --- a/test/commands/geoadd.spec.js.future +++ b/test/commands/geoadd.spec.js @@ -4,7 +4,7 @@ var config = require("../lib/config"); var helper = require("../helper"); var redis = config.redis; -describe("The 'getoadd' method", function () { +describe("The 'geoadd' method", function () { helper.allTests(function(parser, ip, args) { @@ -19,6 +19,7 @@ describe("The 'getoadd' method", function () { }); it('returns 1 if the key exists', function (done) { + helper.serverVersionAtLeast.call(this, client, [3, 2, 0]); client.geoadd("mycity:21:0:location", "13.361389","38.115556","COR", function(err, res) { console.log(err, res); // geoadd is still in the unstable branch. As soon as it reaches the stable one, activate this test diff --git a/test/commands/set.spec.js b/test/commands/set.spec.js index dbdd310..6354f97 100644 --- a/test/commands/set.spec.js +++ b/test/commands/set.spec.js @@ -89,6 +89,16 @@ describe("The 'set' method", function () { }, 100); }); + it("sets the value correctly even if the callback is explicitly set to undefined", function (done) { + client.set(key, value, undefined); + setTimeout(function () { + client.get(key, function (err, res) { + helper.isString(value)(err, res); + done(); + }); + }, 100); + }); + it("sets the value correctly with the array syntax", function (done) { client.set([key, value]); setTimeout(function () { diff --git a/test/commands/zadd.spec.js b/test/commands/zadd.spec.js index 8a7dd2a..973660d 100644 --- a/test/commands/zadd.spec.js +++ b/test/commands/zadd.spec.js @@ -20,12 +20,16 @@ describe("The 'zadd' method", function () { }); it('reports an error', function (done) { + if (helper.redisProcess().spawnFailed()) this.skip(); client.zadd('infinity', [+'5t', "should not be possible"], helper.isError(done)); }); it('return inf / -inf', function (done) { + if (helper.redisProcess().spawnFailed()) this.skip(); + helper.serverVersionAtLeast.call(this, client, [3, 0, 2]); client.zadd('infinity', [+Infinity, "should be inf"], helper.isNumber(1)); - client.zadd('infinity', [-Infinity, "should be negative inf"], helper.isNumber(1)); + client.zadd('infinity', ['inf', 'should be also be inf'], helper.isNumber(1)); + client.zadd('infinity', -Infinity, "should be negative inf", helper.isNumber(1)); client.zadd('infinity', [99999999999999999999999, "should not be inf"], helper.isNumber(1)); client.zrange('infinity', 0, -1, 'WITHSCORES', function (err, res) { assert.equal(res[5], 'inf'); diff --git a/test/detect_buffers.spec.js b/test/detect_buffers.spec.js new file mode 100644 index 0000000..fc9cd1a --- /dev/null +++ b/test/detect_buffers.spec.js @@ -0,0 +1,209 @@ +'use strict'; + +var assert = require("assert"); +var config = require("./lib/config"); +var helper = require('./helper'); +var redis = config.redis; + +describe("detect_buffers", function () { + + helper.allTests(function(parser, ip, args) { + + describe("using " + parser + " and " + ip, function () { + var client; + var args = config.configureClient(parser, ip, { + detect_buffers: true + }); + + beforeEach(function (done) { + client = redis.createClient.apply(redis.createClient, args); + client.once("error", done); + client.once("connect", function () { + client.flushdb(function (err) { + client.hmset("hash key 2", "key 1", "val 1", "key 2", "val 2"); + client.set("string key 1", "string value"); + return done(err); + }); + }); + }); + + describe('get', function () { + describe('first argument is a string', function () { + it('returns a string', function (done) { + client.get("string key 1", helper.isString("string value", done)); + }); + + it('returns a string when executed as part of transaction', function (done) { + client.multi().get("string key 1").exec(helper.isString("string value", done)); + }); + }); + + describe('first argument is a buffer', function () { + it('returns a buffer', function (done) { + client.get(new Buffer("string key 1"), function (err, reply) { + assert.strictEqual(true, Buffer.isBuffer(reply)); + assert.strictEqual("", reply.inspect()); + return done(err); + }); + }); + + it('returns a bufffer when executed as part of transaction', function (done) { + client.multi().get(new Buffer("string key 1")).exec(function (err, reply) { + assert.strictEqual(1, reply.length); + assert.strictEqual(true, Buffer.isBuffer(reply[0])); + assert.strictEqual("", reply[0].inspect()); + return done(err); + }); + }); + }); + }); + + describe('multi.hget', function () { + it('can interleave string and buffer results', function (done) { + client.multi() + .hget("hash key 2", "key 1") + .hget(new Buffer("hash key 2"), "key 1") + .hget("hash key 2", new Buffer("key 2")) + .hget("hash key 2", "key 2") + .exec(function (err, reply) { + assert.strictEqual(true, Array.isArray(reply)); + assert.strictEqual(4, reply.length); + assert.strictEqual("val 1", reply[0]); + assert.strictEqual(true, Buffer.isBuffer(reply[1])); + assert.strictEqual("", reply[1].inspect()); + assert.strictEqual(true, Buffer.isBuffer(reply[2])); + assert.strictEqual("", reply[2].inspect()); + assert.strictEqual("val 2", reply[3]); + return done(err); + }); + }); + }); + + describe('hmget', function () { + describe('first argument is a string', function () { + it('returns strings for keys requested', function (done) { + client.hmget("hash key 2", "key 1", "key 2", function (err, reply) { + assert.strictEqual(true, Array.isArray(reply)); + assert.strictEqual(2, reply.length); + assert.strictEqual("val 1", reply[0]); + assert.strictEqual("val 2", reply[1]); + return done(err); + }); + }); + + it('returns strings for keys requested in transaction', function (done) { + client.multi().hmget("hash key 2", "key 1", "key 2").exec(function (err, reply) { + assert.strictEqual(true, Array.isArray(reply)); + assert.strictEqual(1, reply.length); + assert.strictEqual(2, reply[0].length); + assert.strictEqual("val 1", reply[0][0]); + assert.strictEqual("val 2", reply[0][1]); + return done(err); + }); + }); + + it('handles array of strings with undefined values (repro #344)', function (done) { + client.hmget("hash key 2", "key 3", "key 4", function(err, reply) { + assert.strictEqual(true, Array.isArray(reply)); + assert.strictEqual(2, reply.length); + assert.equal(null, reply[0]); + assert.equal(null, reply[1]); + return done(err); + }); + }); + + it('handles array of strings with undefined values in transaction (repro #344)', function (done) { + client.multi().hmget("hash key 2", "key 3", "key 4").exec(function(err, reply) { + assert.strictEqual(true, Array.isArray(reply)); + assert.strictEqual(1, reply.length); + assert.strictEqual(2, reply[0].length); + assert.equal(null, reply[0][0]); + assert.equal(null, reply[0][1]); + return done(err); + }); + }); + }); + + describe('first argument is a buffer', function () { + it('returns buffers for keys requested', function (done) { + client.hmget(new Buffer("hash key 2"), "key 1", "key 2", function (err, reply) { + assert.strictEqual(true, Array.isArray(reply)); + assert.strictEqual(2, reply.length); + assert.strictEqual(true, Buffer.isBuffer(reply[0])); + assert.strictEqual(true, Buffer.isBuffer(reply[1])); + assert.strictEqual("", reply[0].inspect()); + assert.strictEqual("", reply[1].inspect()); + return done(err); + }); + }); + + it("returns buffers for keys requested in transaction", function (done) { + client.multi().hmget(new Buffer("hash key 2"), "key 1", "key 2").exec(function (err, reply) { + assert.strictEqual(true, Array.isArray(reply)); + assert.strictEqual(1, reply.length); + assert.strictEqual(2, reply[0].length); + assert.strictEqual(true, Buffer.isBuffer(reply[0][0])); + assert.strictEqual(true, Buffer.isBuffer(reply[0][1])); + assert.strictEqual("", reply[0][0].inspect()); + assert.strictEqual("", reply[0][1].inspect()); + return done(err); + }); + }); + }); + }); + + describe('hgetall', function (done) { + describe('first argument is a string', function () { + it('returns string values', function (done) { + client.hgetall("hash key 2", function (err, reply) { + assert.strictEqual("object", typeof reply); + assert.strictEqual(2, Object.keys(reply).length); + assert.strictEqual("val 1", reply["key 1"]); + assert.strictEqual("val 2", reply["key 2"]); + return done(err); + }); + }); + + it('returns string values when executed in transaction', function (done) { + client.multi().hgetall("hash key 2").exec(function (err, reply) { + assert.strictEqual(1, reply.length); + assert.strictEqual("object", typeof reply[0]); + assert.strictEqual(2, Object.keys(reply[0]).length); + assert.strictEqual("val 1", reply[0]["key 1"]); + assert.strictEqual("val 2", reply[0]["key 2"]); + return done(err); + }); + }); + }); + + describe('first argument is a buffer', function () { + it('returns buffer values', function (done) { + client.hgetall(new Buffer("hash key 2"), function (err, reply) { + assert.strictEqual(null, err); + assert.strictEqual("object", typeof reply); + assert.strictEqual(2, Object.keys(reply).length); + assert.strictEqual(true, Buffer.isBuffer(reply["key 1"])); + assert.strictEqual(true, Buffer.isBuffer(reply["key 2"])); + assert.strictEqual("", reply["key 1"].inspect()); + assert.strictEqual("", reply["key 2"].inspect()); + return done(err); + }); + }); + + it('returns buffer values when executed in transaction', function (done) { + client.multi().hgetall(new Buffer("hash key 2")).exec(function (err, reply) { + assert.strictEqual(1, reply.length); + assert.strictEqual("object", typeof reply); + assert.strictEqual(2, Object.keys(reply[0]).length); + assert.strictEqual(true, Buffer.isBuffer(reply[0]["key 1"])); + assert.strictEqual(true, Buffer.isBuffer(reply[0]["key 2"])); + assert.strictEqual("", reply[0]["key 1"].inspect()); + assert.strictEqual("", reply[0]["key 2"].inspect()); + return done(err); + }); + }); + }); + }); + }); + }); +}); \ No newline at end of file diff --git a/test/node_redis.spec.js b/test/node_redis.spec.js index d8d6b0d..f45480e 100644 --- a/test/node_redis.spec.js +++ b/test/node_redis.spec.js @@ -293,12 +293,12 @@ describe("The node_redis client", function () { assert.strictEqual(client.monitoring, false, "monitoring off at start"); client.set("recon 1", "one"); client.monitor(function (err, res) { - assert.strictEqual(client.monitoring, true, "monitoring on after monitor()"); - client.set("recon 2", "two", function (err, res) { - // Do not do this in normal programs. This is to simulate the server closing on us. - // For orderly shutdown in normal programs, do client.quit() - client.stream.destroy(); - }); + assert.strictEqual(client.monitoring, true, "monitoring on after monitor()"); + client.set("recon 2", "two", function (err, res) { + // Do not do this in normal programs. This is to simulate the server closing on us. + // For orderly shutdown in normal programs, do client.quit() + client.stream.destroy(); + }); }); }); @@ -415,203 +415,6 @@ describe("The node_redis client", function () { }); }); - describe('detect_buffers', function () { - var client; - var args = config.configureClient(parser, ip, { - detect_buffers: true - }); - - beforeEach(function (done) { - client = redis.createClient.apply(redis.createClient, args); - client.once("error", done); - client.once("connect", function () { - client.flushdb(function (err) { - client.hmset("hash key 2", "key 1", "val 1", "key 2", "val 2"); - client.set("string key 1", "string value"); - return done(err); - }); - }); - }); - - describe('get', function () { - describe('first argument is a string', function () { - it('returns a string', function (done) { - client.get("string key 1", helper.isString("string value", done)); - }); - - it('returns a string when executed as part of transaction', function (done) { - client.multi().get("string key 1").exec(helper.isString("string value", done)); - }); - }); - - describe('first argument is a buffer', function () { - it('returns a buffer', function (done) { - client.get(new Buffer("string key 1"), function (err, reply) { - assert.strictEqual(true, Buffer.isBuffer(reply)); - assert.strictEqual("", reply.inspect()); - return done(err); - }); - }); - - it('returns a bufffer when executed as part of transaction', function (done) { - client.multi().get(new Buffer("string key 1")).exec(function (err, reply) { - assert.strictEqual(1, reply.length); - assert.strictEqual(true, Buffer.isBuffer(reply[0])); - assert.strictEqual("", reply[0].inspect()); - return done(err); - }); - }); - }); - }); - - describe('multi.hget', function () { - it('can interleave string and buffer results', function (done) { - client.multi() - .hget("hash key 2", "key 1") - .hget(new Buffer("hash key 2"), "key 1") - .hget("hash key 2", new Buffer("key 2")) - .hget("hash key 2", "key 2") - .exec(function (err, reply) { - assert.strictEqual(true, Array.isArray(reply)); - assert.strictEqual(4, reply.length); - assert.strictEqual("val 1", reply[0]); - assert.strictEqual(true, Buffer.isBuffer(reply[1])); - assert.strictEqual("", reply[1].inspect()); - assert.strictEqual(true, Buffer.isBuffer(reply[2])); - assert.strictEqual("", reply[2].inspect()); - assert.strictEqual("val 2", reply[3]); - return done(err); - }); - }); - }); - - describe('hmget', function () { - describe('first argument is a string', function () { - it('returns strings for keys requested', function (done) { - client.hmget("hash key 2", "key 1", "key 2", function (err, reply) { - assert.strictEqual(true, Array.isArray(reply)); - assert.strictEqual(2, reply.length); - assert.strictEqual("val 1", reply[0]); - assert.strictEqual("val 2", reply[1]); - return done(err); - }); - }); - - it('returns strings for keys requested in transaction', function (done) { - client.multi().hmget("hash key 2", "key 1", "key 2").exec(function (err, reply) { - assert.strictEqual(true, Array.isArray(reply)); - assert.strictEqual(1, reply.length); - assert.strictEqual(2, reply[0].length); - assert.strictEqual("val 1", reply[0][0]); - assert.strictEqual("val 2", reply[0][1]); - return done(err); - }); - }); - - it('handles array of strings with undefined values (repro #344)', function (done) { - client.hmget("hash key 2", "key 3", "key 4", function(err, reply) { - assert.strictEqual(true, Array.isArray(reply)); - assert.strictEqual(2, reply.length); - assert.equal(null, reply[0]); - assert.equal(null, reply[1]); - return done(err); - }); - }); - - it('handles array of strings with undefined values in transaction (repro #344)', function (done) { - client.multi().hmget("hash key 2", "key 3", "key 4").exec(function(err, reply) { - assert.strictEqual(true, Array.isArray(reply)); - assert.strictEqual(1, reply.length); - assert.strictEqual(2, reply[0].length); - assert.equal(null, reply[0][0]); - assert.equal(null, reply[0][1]); - return done(err); - }); - }); - }); - - describe('first argument is a buffer', function () { - it('returns buffers for keys requested', function (done) { - client.hmget(new Buffer("hash key 2"), "key 1", "key 2", function (err, reply) { - assert.strictEqual(true, Array.isArray(reply)); - assert.strictEqual(2, reply.length); - assert.strictEqual(true, Buffer.isBuffer(reply[0])); - assert.strictEqual(true, Buffer.isBuffer(reply[1])); - assert.strictEqual("", reply[0].inspect()); - assert.strictEqual("", reply[1].inspect()); - return done(err); - }); - }); - - it("returns buffers for keys requested in transaction", function (done) { - client.multi().hmget(new Buffer("hash key 2"), "key 1", "key 2").exec(function (err, reply) { - assert.strictEqual(true, Array.isArray(reply)); - assert.strictEqual(1, reply.length); - assert.strictEqual(2, reply[0].length); - assert.strictEqual(true, Buffer.isBuffer(reply[0][0])); - assert.strictEqual(true, Buffer.isBuffer(reply[0][1])); - assert.strictEqual("", reply[0][0].inspect()); - assert.strictEqual("", reply[0][1].inspect()); - return done(err); - }); - }); - }); - }); - - describe('hgetall', function (done) { - describe('first argument is a string', function () { - it('returns string values', function (done) { - client.hgetall("hash key 2", function (err, reply) { - assert.strictEqual("object", typeof reply); - assert.strictEqual(2, Object.keys(reply).length); - assert.strictEqual("val 1", reply["key 1"]); - assert.strictEqual("val 2", reply["key 2"]); - return done(err); - }); - }); - - it('returns string values when executed in transaction', function (done) { - client.multi().hgetall("hash key 2").exec(function (err, reply) { - assert.strictEqual(1, reply.length); - assert.strictEqual("object", typeof reply[0]); - assert.strictEqual(2, Object.keys(reply[0]).length); - assert.strictEqual("val 1", reply[0]["key 1"]); - assert.strictEqual("val 2", reply[0]["key 2"]); - return done(err); - }); - }); - }); - - describe('first argument is a buffer', function () { - it('returns buffer values', function (done) { - client.hgetall(new Buffer("hash key 2"), function (err, reply) { - assert.strictEqual(null, err); - assert.strictEqual("object", typeof reply); - assert.strictEqual(2, Object.keys(reply).length); - assert.strictEqual(true, Buffer.isBuffer(reply["key 1"])); - assert.strictEqual(true, Buffer.isBuffer(reply["key 2"])); - assert.strictEqual("", reply["key 1"].inspect()); - assert.strictEqual("", reply["key 2"].inspect()); - return done(err); - }); - }); - - it('returns buffer values when executed in transaction', function (done) { - client.multi().hgetall(new Buffer("hash key 2")).exec(function (err, reply) { - assert.strictEqual(1, reply.length); - assert.strictEqual("object", typeof reply); - assert.strictEqual(2, Object.keys(reply[0]).length); - assert.strictEqual(true, Buffer.isBuffer(reply[0]["key 1"])); - assert.strictEqual(true, Buffer.isBuffer(reply[0]["key 2"])); - assert.strictEqual("", reply[0]["key 1"].inspect()); - assert.strictEqual("", reply[0]["key 2"].inspect()); - return done(err); - }); - }); - }); - }); - }); - describe('unref', function () { it('exits subprocess as soon as final command is processed', function (done) { var args = config.HOST[ip] ? [config.HOST[ip], config.PORT] : [ip];