redis/node-redis
1
0
Fork 0
mirror of https://github.com/redis/node-redis.git synced 2025-08-04 15:02:09 +03:00
Code Activity

Fix inconsistent command argument handling

Browse Source 
Earlier multi.command and client.command diverged a lot in the way they accepted arguments. This is now consistent
This will also fix some bugs like using multi.hmset with arrays
This commit is contained in:
Ruben Bridgewater
2015-09-14 15:07:32 +02:00
parent e24f056b2d
commit c522ca1264
13 changed files with 208 additions and 76 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
.jshintrc
View File

@@ -16,6 +16,9 @@
"node": true, // Enable globals available when code is running inside of the NodeJS runtime environment.
"mocha": true,
// Relaxing options
"boss": true, // Accept things like `while (command = keys.shift()) { ... }`
"overrides": {
"examples/*.js": {
"unused": false

127
index.js
View File

@@ -863,8 +863,16 @@ RedisClient.prototype.end = function () {
function Multi(client, args) {
this._client = client;
this.queue = [["multi"]];
var command, tmp_args;
if (Array.isArray(args)) {
this.queue = this.queue.concat(args);
while (tmp_args = args.shift()) {
command = tmp_args.shift();
if (Array.isArray(command)) {
this[command[0]].apply(this, command.slice(1).concat(tmp_args));
} else {
this[command].apply(this, tmp_args);
}
}
}
}
@@ -878,16 +886,32 @@ commands.forEach(function (fullCommand) {
return;
}
RedisClient.prototype[command] = function (args, callback) {
if (Array.isArray(args)) {
return this.send_command(command, args, callback);
RedisClient.prototype[command] = function (key, arg, callback) {
if (Array.isArray(key)) {
return this.send_command(command, key, arg);
}
if (Array.isArray(arg)) {
arg.unshift(key);
return this.send_command(command, arg, callback);
}
return this.send_command(command, to_array(arguments));
};
RedisClient.prototype[command.toUpperCase()] = RedisClient.prototype[command];
Multi.prototype[command] = function () {
this.queue.push([command].concat(to_array(arguments)));
Multi.prototype[command] = function (key, arg, callback) {
if (Array.isArray(key)) {
if (arg) {
key.push(arg);
}
this.queue.push([command].concat(key));
} else if (Array.isArray(arg)) {
if (callback) {
arg.push(callback);
}
this.queue.push([command, key].concat(arg));
} else {
this.queue.push([command].concat(to_array(arguments)));
}
return this;
};
Multi.prototype[command.toUpperCase()] = Multi.prototype[command];
@@ -919,70 +943,63 @@ RedisClient.prototype.auth = RedisClient.prototype.AUTH = function (pass, callba
}
};
RedisClient.prototype.hmget = RedisClient.prototype.HMGET = function (arg1, arg2, arg3) {
if (Array.isArray(arg2) && typeof arg3 === "function") {
return this.send_command("hmget", [arg1].concat(arg2), arg3);
} else if (Array.isArray(arg1) && typeof arg2 === "function") {
return this.send_command("hmget", arg1, arg2);
} else {
return this.send_command("hmget", to_array(arguments));
RedisClient.prototype.hmset = RedisClient.prototype.HMSET = function (key, args, callback) {
var field, tmp_args;
if (Array.isArray(key)) {
return this.send_command("hmset", key, args);
}
};
RedisClient.prototype.hmset = RedisClient.prototype.HMSET = function (args, callback) {
var tmp_args, tmp_keys, i, il, key;
if (Array.isArray(args)) {
return this.send_command("hmset", args, callback);
return this.send_command("hmset", [key].concat(args), callback);
}
args = to_array(arguments);
if (typeof args[args.length - 1] === "function") {
callback = args[args.length - 1];
args.length -= 1;
} else {
callback = null;
}
if (args.length === 2 && (typeof args[0] === "string" || typeof args[0] === "number") && typeof args[1] === "object") {
if (typeof args === "object") {
// User does: client.hmset(key, {key1: val1, key2: val2})
// assuming key is a string, i.e. email address
// if key is a number, i.e. timestamp, convert to string
if (typeof args[0] === "number") {
args[0] = args[0].toString();
// TODO: This seems random and no other command get's the key converted => either all or none should behave like this
if (typeof key !== "string") {
key = key.toString();
}
tmp_args = [ args[0] ];
tmp_keys = Object.keys(args[1]);
for (i = 0, il = tmp_keys.length; i < il ; i++) {
key = tmp_keys[i];
tmp_args.push(key);
tmp_args.push(args[1][key]);
tmp_args = [key];
var fields = Object.keys(args);
while (field = fields.shift()) {
tmp_args.push(field, args[field]);
}
args = tmp_args;
return this.send_command("hmset", tmp_args, callback);
}
return this.send_command("hmset", args, callback);
return this.send_command("hmset", to_array(arguments));
};
Multi.prototype.hmset = Multi.prototype.HMSET = function () {
var args = to_array(arguments), tmp_args;
if (args.length >= 2 && typeof args[0] === "string" && typeof args[1] === "object") {
tmp_args = [ "hmset", args[0] ];
Object.keys(args[1]).map(function (key) {
tmp_args.push(key);
tmp_args.push(args[1][key]);
});
if (args[2]) {
tmp_args.push(args[2]);
Multi.prototype.hmset = Multi.prototype.HMSET = function (key, args, callback) {
var tmp_args, field;
if (Array.isArray(key)) {
if (args) {
key.push(args);
}
tmp_args = ['hmset'].concat(key);
} else if (Array.isArray(args)) {
if (callback) {
args.push(callback);
}
tmp_args = ['hmset', key].concat(args);
} else if (typeof args === "object") {
tmp_args = ["hmset", key];
if (typeof key !== "string") {
key = key.toString();
}
var fields = Object.keys(args);
while (field = fields.shift()) {
tmp_args.push(field);
tmp_args.push(args[field]);
}
if (callback) {
tmp_args.push(callback);
}
args = tmp_args;
} else {
args.unshift("hmset");
tmp_args = to_array(arguments);
tmp_args.unshift("hmset");
}
this.queue.push(args);
this.queue.push(tmp_args);
return this;
};

10
test/commands/blpop.spec.js
View File

@@ -33,6 +33,16 @@ describe("The 'blpop' method", function () {
});
});
it('pops value immediately if list contains values using array notation', function (done) {
bclient = redis.createClient.apply(redis.createClient, args);
client.rpush(["blocking list", "initial value"], helper.isNumber(1));
bclient.blpop(["blocking list", 0], function (err, value) {
assert.strictEqual(value[0], "blocking list");
assert.strictEqual(value[1], "initial value");
return done(err);
});
});
it('waits for value if list is not yet populated', function (done) {
bclient = redis.createClient.apply(redis.createClient, args);
bclient.blpop("blocking list 2", 5, function (err, value) {

11
test/commands/client.spec.js
View File

@@ -37,8 +37,17 @@ describe("The 'client' method", function () {
});
});
it("lists connected clients when invoked with array syntax on client", function (done) {
client.multi().client(["list"]).exec(function(err, results) {
assert(pattern.test(results[0]), "expected string '" + results + "' to match " + pattern.toString());
return done();
});
});
it("lists connected clients when invoked with multi's array syntax", function (done) {
client.multi().client("list").exec(function(err, results) {
client.multi([
['client', 'list']
]).exec(function(err, results) {
assert(pattern.test(results[0]), "expected string '" + results + "' to match " + pattern.toString());
return done();
});

2
test/commands/dbsize.spec.js
View File

@@ -71,7 +71,7 @@ describe("The 'dbsize' method", function () {
var oldSize;
beforeEach(function (done) {
client.dbsize([], function (err, res) {
client.dbsize(function (err, res) {
helper.isType.number()(err, res);
assert.strictEqual(res, 0, "Initial db size should be 0");

14
test/commands/del.spec.js
View File

@@ -36,6 +36,20 @@ describe("The 'del' method", function () {
client.get('apple', helper.isNull(done));
});
it('allows multiple keys to be deleted with the array syntax', function (done) {
client.mset('foo', 'bar', 'apple', 'banana');
client.del(['foo', 'apple'], helper.isNumber(2));
client.get('foo', helper.isNull());
client.get('apple', helper.isNull(done));
});
it('allows multiple keys to be deleted with the array syntax and no callback', function (done) {
client.mset('foo', 'bar', 'apple', 'banana');
client.del(['foo', 'apple']);
client.get('foo', helper.isNull());
client.get('apple', helper.isNull(done));
});
afterEach(function () {
client.end();
});

5
test/commands/exits.spec.js
View File

@@ -24,6 +24,11 @@ describe("The 'exits' method", function () {
client.EXISTS('foo', helper.isNumber(1, done));
});
it('returns 1 if the key exists with array syntax', function (done) {
client.set('foo', 'bar');
client.EXISTS(['foo'], helper.isNumber(1, done));
});
it('returns 0 if the key does not exist', function (done) {
client.exists('bar', helper.isNumber(0, done));
});

8
test/commands/expire.spec.js
View File

@@ -20,6 +20,14 @@ describe("The 'expire' method", function () {
});
it('expires key after timeout', function (done) {
client.set(['expiry key', 'bar'], helper.isString("OK"));
client.EXPIRE("expiry key", "1", helper.isNumber(1));
setTimeout(function () {
client.exists(["expiry key"], helper.isNumber(0, done));
}, 1100);
});
it('expires key after timeout with array syntax', function (done) {
client.set(['expiry key', 'bar'], helper.isString("OK"));
client.EXPIRE(["expiry key", "1"], helper.isNumber(1));
setTimeout(function () {

48
test/commands/flushdb.spec.js
View File

@@ -58,36 +58,50 @@ describe("The 'flushdb' method", function () {
describe("when there is data in Redis", function () {
beforeEach(function (done) {
var end = helper.callFuncAfter(function () {
client.flushdb(helper.isString("OK", done));
}, 2);
client.mset(key, uuid.v4(), key2, uuid.v4(), helper.isNotError(end));
client.mset(key, uuid.v4(), key2, uuid.v4(), helper.isNotError());
client.dbsize([], function (err, res) {
helper.isType.positiveNumber()(err, res);
assert.equal(res, 2, 'Two keys should have been inserted');
end();
done();
});
});
it("deletes all the keys", function (done) {
client.mget(key, key2, function (err, res) {
assert.strictEqual(null, err, "Unexpected error returned");
assert.strictEqual(true, Array.isArray(res), "Results object should be an array.");
assert.strictEqual(2, res.length, "Results array should have length 2.");
assert.strictEqual(null, res[0], "Redis key should have been flushed.");
assert.strictEqual(null, res[1], "Redis key should have been flushed.");
done(err);
client.flushdb(function(err, res) {
assert.equal(res, 'OK');
client.mget(key, key2, function (err, res) {
assert.strictEqual(null, err, "Unexpected error returned");
assert.strictEqual(true, Array.isArray(res), "Results object should be an array.");
assert.strictEqual(2, res.length, "Results array should have length 2.");
assert.strictEqual(null, res[0], "Redis key should have been flushed.");
assert.strictEqual(null, res[1], "Redis key should have been flushed.");
done(err);
});
});
});
it("results in a db size of zero", function (done) {
client.dbsize([], function (err, res) {
helper.isNotError()(err, res);
helper.isType.number()(err, res);
assert.strictEqual(0, res, "Flushing db should result in db size 0");
done();
client.flushdb(function(err, res) {
client.dbsize([], function (err, res) {
helper.isNotError()(err, res);
helper.isType.number()(err, res);
assert.strictEqual(0, res, "Flushing db should result in db size 0");
done();
});
});
});
it("results in a db size of zero without a callback", function (done) {
client.flushdb();
setTimeout(function(err, res) {
client.dbsize([], function (err, res) {
helper.isNotError()(err, res);
helper.isType.number()(err, res);
assert.strictEqual(0, res, "Flushing db should result in db size 0");
done();
});
}, 25);
});
});
});
});

15
test/commands/get.spec.js
View File

@@ -70,6 +70,21 @@ describe("The 'get' method", function () {
done(err);
});
});
it("gets the value correctly with array syntax and the callback being in the array", function (done) {
client.GET([key, function (err, res) {
helper.isString(value)(err, res);
done(err);
}]);
});
it("should not throw on a get without callback (even if it's not useful", function (done) {
client.GET(key);
client.on('error', function(err) {
throw err;
});
setTimeout(done, 50);
});
});
describe("when the key does not exist in Redis", function () {

20
test/commands/getset.spec.js
View File

@@ -74,6 +74,26 @@ describe("The 'getset' method", function () {
});
});
});
it("gets the value correctly with array syntax", function (done) {
client.GETSET([key, value2], function (err, res) {
helper.isString(value)(err, res);
client.get(key, function (err, res) {
helper.isString(value2)(err, res);
done(err);
});
});
});
it("gets the value correctly with array syntax style 2", function (done) {
client.GETSET(key, [value2], function (err, res) {
helper.isString(value)(err, res);
client.get(key, function (err, res) {
helper.isString(value2)(err, res);
done(err);
});
});
});
});
describe("when the key does not exist in Redis", function () {

4
test/commands/hgetall.spec.js
View File

@@ -67,7 +67,7 @@ describe("The 'hgetall' method", function () {
it('returns binary results', function (done) {
client.hmset(["bhosts", "mjr", "1", "another", "23", "home", "1234", new Buffer([0xAA, 0xBB, 0x00, 0xF0]), new Buffer([0xCC, 0xDD, 0x00, 0xF0])], helper.isString("OK"));
client.HGETALL(["bhosts"], function (err, obj) {
client.HGETALL(["bhosts", function (err, obj) {
assert.strictEqual(4, Object.keys(obj).length);
assert.strictEqual("1", obj.mjr.toString());
assert.strictEqual("23", obj.another.toString());
@@ -75,7 +75,7 @@ describe("The 'hgetall' method", function () {
assert.strictEqual((new Buffer([0xAA, 0xBB, 0x00, 0xF0])).toString('binary'), Object.keys(obj)[3]);
assert.strictEqual((new Buffer([0xCC, 0xDD, 0x00, 0xF0])).toString('binary'), obj[(new Buffer([0xAA, 0xBB, 0x00, 0xF0])).toString('binary')].toString('binary'));
return done(err);
});
}]);
});
});

17
test/commands/multi.spec.js
View File

@@ -165,6 +165,23 @@ describe("The 'multi' method", function () {
});
});
it('allows multiple commands to work the same as normal to be performed using a chaining API', function (done) {
client.multi()
.mset(['some', '10', 'keys', '20'])
.incr('some')
.incr('keys')
.mget('some', 'keys')
.exec(function (err, replies) {
assert.strictEqual(null, err);
assert.equal('OK', replies[0]);
assert.equal(11, replies[1]);
assert.equal(21, replies[2]);
assert.equal(11, replies[3][0].toString());
assert.equal(21, replies[3][1].toString());
return done();
});
});
it('allows an array to be provided indicating multiple operations to perform', function (done) {
// test nested multi-bulk replies with nulls.
client.multi([