Skip to content

Commit 6958c18

Browse files
author
Ruben Bridgewater
committed
Increase the coverage by adding tests and fix a failing ready check
1 parent 6975b07 commit 6958c18

File tree

5 files changed

+77
-42
lines changed

5 files changed

+77
-42
lines changed

‎index.js

Lines changed: 10 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -337,18 +337,19 @@ RedisClient.prototype.on_ready = function () {
337337
};
338338

339339
RedisClient.prototype.on_info_cmd = function (err, res) {
340+
if (err) {
341+
err.message = "Ready check failed: " + err.message;
342+
this.emit("error", err);
343+
return;
344+
}
345+
340346
var self = this;
341347
var obj = {};
342348
var lines = res.toString().split("\r\n");
343349
var i = 0;
344350
var key = 'db' + i;
345351
var line, retry_time, parts, sub_parts;
346352

347-
if (err) {
348-
err.message = "Ready check failed: " + err.message;
349-
return self.emit("error", err);
350-
}
351-
352353
for (i = 0; i < lines.length; i++) {
353354
parts = lines[i].split(':');
354355
if (parts[1]) {
@@ -369,9 +370,7 @@ RedisClient.prototype.on_info_cmd = function (err, res) {
369370
obj[key] = {};
370371
while (line = parts.pop()) {
371372
sub_parts = line.split('=');
372-
if (sub_parts[1]) {
373-
obj[key][sub_parts[0]] = +sub_parts[1];
374-
}
373+
obj[key][sub_parts[0]] = +sub_parts[1];
375374
}
376375
i++;
377376
key = 'db' + i;
@@ -471,7 +470,7 @@ RedisClient.prototype.connection_gone = function (why) {
471470

472471
// If this is a requested shutdown, then don't retry
473472
if (this.closing) {
474-
debug("connection ended from quit command, not retrying.");
473+
debug("Connection ended from quit command, not retrying.");
475474
this.flush_and_error(new Error("Redis connection gone from " + why + " event."));
476475
return;
477476
}
@@ -656,7 +655,7 @@ RedisClient.prototype.return_reply = function (reply) {
656655
this.emit("monitor", timestamp, args);
657656
} else {
658657
var err = new Error("node_redis command queue state error. If you can reproduce this, please report it.");
659-
err.command = command_obj.command.toUpperCase();
658+
err.command_obj = command_obj;
660659
this.emit("error", err);
661660
}
662661
};
@@ -695,19 +694,8 @@ RedisClient.prototype.send_command = function (command, args, callback) {
695694
callback = process.domain.bind(callback);
696695
}
697696

698-
// if the last argument is an array and command is sadd or srem, expand it out:
699-
// client.sadd(arg1, [arg2, arg3, arg4], cb);
700-
// converts to:
701-
// client.sadd(arg1, arg2, arg3, arg4, cb);
702-
if ((command === 'sadd' || command === 'srem') && args.length > 0 && Array.isArray(args[args.length - 1])) {
703-
args = args.slice(0, -1).concat(args[args.length - 1]);
704-
}
705-
706-
// if the value is undefined or null and command is set or setx, need not to send message to redis
707697
if (command === 'set' || command === 'setex') {
708-
if (args.length === 0) {
709-
return;
710-
}
698+
// if the value is undefined or null and command is set or setx, need not to send message to redis
711699
if (args[args.length - 1] === undefined || args[args.length - 1] === null) {
712700
command = command.toUpperCase();
713701
err = new Error('send_command: ' + command + ' value must not be undefined or null');

‎test/auth.spec.js

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,34 @@ describe("client authentication", function () {
159159
client.auth(auth, helper.isString('OK', done));
160160
});
161161
});
162+
163+
it('does not allow any commands to be processed if not authenticated using no_ready_check true', function (done) {
164+
if (helper.redisProcess().spawnFailed()) this.skip();
165+
166+
var args = config.configureClient(parser, ip, {
167+
no_ready_check: true
168+
});
169+
client = redis.createClient.apply(redis.createClient, args);
170+
client.on("ready", function () {
171+
client.set('foo', 'bar', function (err, res) {
172+
assert.equal(err.message, 'NOAUTH Authentication required.');
173+
assert.equal(err.code, 'NOAUTH');
174+
assert.equal(err.command, 'SET');
175+
done();
176+
});
177+
});
178+
});
179+
180+
it('does not allow auth to be provided post-hoc with auth method if not authenticated before', function (done) {
181+
if (helper.redisProcess().spawnFailed()) this.skip();
182+
client = redis.createClient.apply(redis.createClient, args);
183+
client.on("error", function (err) {
184+
assert.equal(err.code, 'NOAUTH');
185+
assert.equal(err.message, 'Ready check failed: NOAUTH Authentication required.');
186+
assert.equal(err.command, 'INFO');
187+
done();
188+
});
189+
});
162190
});
163191
});
164192

‎test/commands/set.spec.js

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ describe("The 'set' method", function () {
7070
it("reports an error", function (done) {
7171
client.set(undefined, function (err, res) {
7272
helper.isError()(err, null);
73+
assert.equal(err.command, 'SET');
7374
done();
7475
});
7576
});
@@ -90,28 +91,35 @@ describe("The 'set' method", function () {
9091
});
9192

9293
describe("with undefined 'key' and missing 'value' parameter", function () {
93-
it("does not emit an error", function (done) {
94-
this.timeout(200);
95-
96-
client.once("error", function (err) {
97-
done(err);
94+
it("emits an error without callback", function (done) {
95+
client.on('error', function (err) {
96+
assert.equal(err.message, 'send_command: SET value must not be undefined or null');
97+
assert.equal(err.command, 'SET');
98+
done();
9899
});
100+
client.set(undefined);
101+
});
102+
});
99103

100-
client.set();
101-
102-
setTimeout(function () {
103-
done();
104-
}, 100);
104+
it("emit an error with only the key set", function (done) {
105+
client.on('error', function (err) {
106+
assert.equal(err.message, "ERR wrong number of arguments for 'set' command");
107+
done();
105108
});
106109

107-
it("does emit an error", function (done) {
108-
client.on('error', function (err) {
109-
assert.equal(err.message, "ERR wrong number of arguments for 'set' command");
110-
done();
111-
});
110+
client.set('foo');
111+
});
112112

113-
client.set('foo');
113+
it("emit an error without any parameters", function (done) {
114+
client.once("error", function (err) {
115+
assert.equal(err.message, 'send_command: SET value must not be undefined or null');
116+
assert.equal(err.command, 'SET');
117+
done();
114118
});
119+
120+
// This was not supported not to throw earlier and was added by the test refactoring
121+
// https://github.com/NodeRedis/node_redis/commit/eaca486ab1aecd1329f7452ad2f2255b1263606f
122+
client.set();
115123
});
116124
});
117125
});

‎test/commands/srem.spec.js

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,16 @@ describe("The 'srem' method", function () {
3939
});
4040
});
4141

42+
it('allows multiple values to be removed with send_command', function (done) {
43+
client.send_command('sadd', ['set0', 'member0', 'member1', 'member2'], helper.isNumber(3));
44+
client.send_command('srem', ["set0", "member1", "member2"], helper.isNumber(2));
45+
client.smembers("set0", function (err, res) {
46+
assert.strictEqual(res.length, 1);
47+
assert.ok(~res.indexOf("member0"));
48+
return done(err);
49+
});
50+
});
51+
4252
it('handles a value missing from the set of values being removed', function (done) {
4353
client.sadd(["set0", "member0", "member1", "member2"], helper.isNumber(3));
4454
client.SREM(["set0", "member3", "member4"], helper.isNumber(0));

‎test/node_redis.spec.js

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -829,17 +829,18 @@ describe("The node_redis client", function () {
829829
var cb = function(err, reply) {
830830
assert.equal(err.code, 'CONNECTION_BROKEN');
831831
};
832-
for (var i = 0; i < 10; i += 2) {
833-
multi.set("foo" + i, "bar" + i);
832+
for (var i = 0; i < 12; i += 3) {
833+
client.set("foo" + i, "bar" + i);
834834
multi.set("foo" + (i + 1), "bar" + (i + 1), cb);
835+
multi.set("foo" + (i + 2), "bar" + (i + 2));
835836
}
836837
multi.exec();
837-
assert.equal(client.command_queue.length, 13);
838+
assert.equal(client.command_queue.length, 15);
838839
helper.killConnection(client);
839840
});
840841

841842
client.on("reconnecting", function (params) {
842-
assert.equal(client.command_queue.length, 13);
843+
assert.equal(client.command_queue.length, 15);
843844
});
844845

845846
client.on('error', function(err) {

0 commit comments

Comments
 (0)