From 005e869d838635ed1a4e640231f3181b12c8b2f2 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Fri, 4 Sep 2015 15:14:57 +0200 Subject: [PATCH] Remove send_command safety checks This checks are only important for users who use send_command directly instead of using the convience method. As the readme clearly stats how send_command should work and any user would have run into errors if misused, these checks can be removed. If any user might misuse the function anyway, it is very likely that another error will be thrown because of that Fix #629 and insert tests --- index.js | 45 ++++++++++++++++------------------------- test/node_redis.spec.js | 38 ++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 28 deletions(-) diff --git a/index.js b/index.js index efca586644..ec8e22dd82 100644 --- a/index.js +++ b/index.js @@ -661,37 +661,26 @@ function Command(command, args, sub_command, buffer_args, callback) { } 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, last_arg_type; + var arg, command_obj, i, elem_count, buffer_args, stream = this.stream, command_str = "", buffered_writes = 0; - if (typeof command !== "string") { - throw new Error("First argument to send_command must be the command name string, not " + typeof command); + // 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(); } - if (Array.isArray(args)) { - if (typeof callback === "function") { - // probably the fastest way: - // client.command([arg1, arg2], cb); (straight passthrough) - // send_command(command, [arg1, arg2], cb); - } else if (!callback) { - // 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]); - last_arg_type = typeof args[args.length - 1]; - if (last_arg_type === "function" || last_arg_type === "undefined") { - callback = args.pop(); - } - } else { - throw new Error("send_command: last argument must be a callback or undefined"); - } - } else { - throw new Error("send_command: second argument must be an array"); - } - - if (callback && process.domain) { + if (process.domain && callback) { callback = process.domain.bind(callback); } diff --git a/test/node_redis.spec.js b/test/node_redis.spec.js index e9eb0c3359..bad7439687 100644 --- a/test/node_redis.spec.js +++ b/test/node_redis.spec.js @@ -117,6 +117,44 @@ describe("The node_redis client", function () { client.end(); }); + describe("send_command", function () { + + it("omitting args should be fine in some cases", function (done) { + client.send_command("info", undefined, function(err, res) { + assert(/redis_version/.test(res)); + done(); + }); + }); + + it("using another type as cb should just work as if there were no callback parameter", function (done) { + client.send_command('set', ['test', 'bla'], [true]); + client.get('test', function(err, res) { + assert.equal(res, 'bla'); + done(); + }); + }); + + it("misusing the function should eventually throw (no command)", function (done) { + var mochaListener = helper.removeMochaListener(); + + process.once('uncaughtException', function (err) { + process.on('uncaughtException', mochaListener); + assert(/is not a function|toUpperCase/.test(err)); + done(); + }); + + client.send_command(true, 'info'); + }); + + it("misusing the function should eventually throw (wrong args)", function (done) { + client.send_command('info', false, function(err, res) { + assert.equal(err.message, 'ERR Protocol error: invalid multibulk length'); + done(); + }); + }); + + }); + describe("when redis closes unexpectedly", function () { it("reconnects and can retrieve the pre-existing data", function (done) { client.on("reconnecting", function on_recon(params) {