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

Minor changes

Browse Source 
Move utility functions in lib/utils.js
Improve the js parser in cases the buffer is incomplete
Rename lib/parser to lib/parsers

Fix smaller issues with test suite and fix parser errors not being catched

Fixed wrong test for the new .end flush parameter
Fixed test suite options being partly mutated
Add some more tests
This commit is contained in:
Ruben Bridgewater
2015-09-25 00:54:16 +02:00
parent ff47dc3ce8
commit 5f261c5823
13 changed files with 212 additions and 102 deletions
Download Patch File Download Diff File Expand all files Collapse all files

64
index.js
View File

@@ -3,8 +3,8 @@
var net = require("net"),
URL = require("url"),
util = require("util"),
utils = require("./lib/utils"),
Queue = require("./lib/queue"),
to_array = require("./lib/to_array"),
events = require("events"),
parsers = [],
// This static list of commands is updated from time to time.
@@ -23,14 +23,13 @@ exports.debug_mode = /\bredis\b/i.test(process.env.NODE_DEBUG);
// hiredis might not be installed
try {
require("./lib/parser/hiredis");
parsers.push(require("./lib/parser/hiredis"));
parsers.push(require("./lib/parsers/hiredis"));
} catch (err) {
/* istanbul ignore next: won't be reached with tests */
debug("Hiredis parser not installed.");
}
parsers.push(require("./lib/parser/javascript"));
parsers.push(require("./lib/parsers/javascript"));
function RedisClient(stream, options) {
options = options || {};
@@ -135,6 +134,7 @@ RedisClient.prototype.flush_and_error = function (error) {
while (command_obj = this.offline_queue.shift()) {
if (typeof command_obj.callback === "function") {
error.command = command_obj.command.toUpperCase();
command_obj.callback(error);
}
}
@@ -142,6 +142,7 @@ RedisClient.prototype.flush_and_error = function (error) {
while (command_obj = this.command_queue.shift()) {
if (typeof command_obj.callback === "function") {
error.command = command_obj.command.toUpperCase();
command_obj.callback(error);
}
}
@@ -530,41 +531,6 @@ RedisClient.prototype.return_error = function (err) {
}
};
// hgetall converts its replies to an Object. If the reply is empty, null is returned.
function reply_to_object(reply) {
var obj = {}, j, jl, key, val;
if (reply.length === 0 || !Array.isArray(reply)) {
return null;
}
for (j = 0, jl = reply.length; j < jl; j += 2) {
key = reply[j].toString('binary');
val = reply[j + 1];
obj[key] = val;
}
return obj;
}
function reply_to_strings(reply) {
var i;
if (Buffer.isBuffer(reply)) {
return reply.toString();
}
if (Array.isArray(reply)) {
for (i = 0; i < reply.length; i++) {
// Recusivly call the function as slowlog returns deep nested replies
reply[i] = reply_to_strings(reply[i]);
}
return reply;
}
return reply;
}
RedisClient.prototype.return_reply = function (reply) {
var command_obj, len, type, timestamp, argindex, args, queue_len;
@@ -598,12 +564,12 @@ RedisClient.prototype.return_reply = function (reply) {
if (this.options.detect_buffers && command_obj.buffer_args === false) {
// If detect_buffers option was specified, then the reply from the parser will be Buffers.
// If this command did not use Buffer arguments, then convert the reply to Strings here.
reply = reply_to_strings(reply);
reply = utils.reply_to_strings(reply);
}
// TODO - confusing and error-prone that hgetall is special cased in two places
if (reply && 'hgetall' === command_obj.command) {
reply = reply_to_object(reply);
reply = utils.reply_to_object(reply);
}
}
@@ -614,7 +580,7 @@ RedisClient.prototype.return_reply = function (reply) {
} else if (this.pub_sub_mode || command_obj && command_obj.sub_command) {
if (Array.isArray(reply)) {
if (!this.options.return_buffers && (!command_obj || this.options.detect_buffers && command_obj.buffer_args === false)) {
reply = reply_to_strings(reply);
reply = utils.reply_to_strings(reply);
}
type = reply[0].toString();
@@ -850,7 +816,7 @@ RedisClient.prototype.end = function (flush) {
// Flush queue if wanted
if (flush) {
this.flush_and_error("Redis connection ended.");
this.flush_and_error(new Error("The command can't be processed. The connection has already been closed."));
}
this.connected = false;
@@ -894,7 +860,7 @@ commands.forEach(function (fullCommand) {
arg = [key].concat(arg);
return this.send_command(command, arg, callback);
}
return this.send_command(command, to_array(arguments));
return this.send_command(command, utils.to_array(arguments));
};
RedisClient.prototype[command.toUpperCase()] = RedisClient.prototype[command];
@@ -910,7 +876,7 @@ commands.forEach(function (fullCommand) {
}
this.queue.push([command, key].concat(arg));
} else {
this.queue.push([command].concat(to_array(arguments)));
this.queue.push([command].concat(utils.to_array(arguments)));
}
return this;
};
@@ -979,7 +945,7 @@ RedisClient.prototype.hmset = RedisClient.prototype.HMSET = function (key, args,
}
return this.send_command("hmset", tmp_args, callback);
}
return this.send_command("hmset", to_array(arguments));
return this.send_command("hmset", utils.to_array(arguments));
};
Multi.prototype.hmset = Multi.prototype.HMSET = function (key, args, callback) {
@@ -1008,7 +974,7 @@ Multi.prototype.hmset = Multi.prototype.HMSET = function (key, args, callback) {
tmp_args.push(callback);
}
} else {
tmp_args = to_array(arguments);
tmp_args = utils.to_array(arguments);
tmp_args.unshift("hmset");
}
this.queue.push(tmp_args);
@@ -1087,11 +1053,11 @@ Multi.prototype.execute_callback = function (err, replies) {
replies[i].command = args[0].toUpperCase();
} else if (replies[i]) {
if (this._client.options.detect_buffers && this.wants_buffers[i + 1] === false) {
replies[i] = reply_to_strings(replies[i]);
replies[i] = utils.reply_to_strings(replies[i]);
}
if (args[0] === "hgetall") {
// TODO - confusing and error-prone that hgetall is special cased in two places
replies[i] = reply_to_object(replies[i]);
replies[i] = utils.reply_to_object(replies[i]);
}
}

8
lib/parser/hiredis.js → lib/parsers/hiredis.js
View File

@@ -18,7 +18,13 @@ HiredisReplyParser.prototype.execute = function (data) {
var reply;
this.reader.feed(data);
while (true) {
reply = this.reader.get();
try {
reply = this.reader.get();
} catch (err) {
// Protocol errors land here
this.send_error(err);
break;
}
if (reply === undefined) {
break;

26
lib/parser/javascript.js → lib/parsers/javascript.js
View File

@@ -30,14 +30,13 @@ ReplyParser.prototype._parseResult = function (type) {
end = this._packetEndOffset() - 1;
start = this._offset;
// include the delimiter
this._offset = end + 2;
if (end > this._buffer.length) {
this._offset = start;
throw new IncompleteReadBuffer("Wait for more data.");
}
// include the delimiter
this._offset = end + 2;
if (type === 45) {
return new Error(this._buffer.toString(this._encoding, start, end));
} else if (this.return_buffers) {
@@ -49,14 +48,13 @@ ReplyParser.prototype._parseResult = function (type) {
end = this._packetEndOffset() - 1;
start = this._offset;
// include the delimiter
this._offset = end + 2;
if (end > this._buffer.length) {
this._offset = start;
throw new IncompleteReadBuffer("Wait for more data.");
}
// include the delimiter
this._offset = end + 2;
// return the coerced numeric value
return +this._buffer.toString('ascii', start, end);
} else if (type === 36) { // $
@@ -74,14 +72,13 @@ ReplyParser.prototype._parseResult = function (type) {
end = this._offset + packetHeader.size;
start = this._offset;
// set the offset to after the delimiter
this._offset = end + 2;
if (end > this._buffer.length) {
this._offset = offset;
throw new IncompleteReadBuffer("Wait for more data.");
}
// set the offset to after the delimiter
this._offset = end + 2;
if (this.return_buffers) {
return this._buffer.slice(start, end);
}
@@ -157,6 +154,11 @@ ReplyParser.prototype.execute = function (buffer) {
ret = this._parseResult(type);
this.send_reply(ret);
} else if (type === 10 || type === 13) {
break;
} else {
var err = new Error('Protocol error, got "' + String.fromCharCode(type) + '" as reply type byte');
this.send_error(err);
}
} catch (err) {
// catch the error (not enough data), rewind, and wait

14
lib/to_array.js
View File

@@ -1,14 +0,0 @@
'use strict';
function to_array(args) {
var len = args.length,
arr = new Array(len), i;
for (i = 0; i < len; i += 1) {
arr[i] = args[i];
}
return arr;
}
module.exports = to_array;

53
lib/utils.js Normal file
View File

@@ -0,0 +1,53 @@
'use strict';
// hgetall converts its replies to an Object. If the reply is empty, null is returned.
function replyToObject(reply) {
var obj = {}, j, jl, key, val;
if (reply.length === 0 || !Array.isArray(reply)) {
return null;
}
for (j = 0, jl = reply.length; j < jl; j += 2) {
key = reply[j].toString('binary');
val = reply[j + 1];
obj[key] = val;
}
return obj;
}
function replyToStrings(reply) {
var i;
if (Buffer.isBuffer(reply)) {
return reply.toString();
}
if (Array.isArray(reply)) {
for (i = 0; i < reply.length; i++) {
// Recusivly call the function as slowlog returns deep nested replies
reply[i] = replyToStrings(reply[i]);
}
return reply;
}
return reply;
}
function toArray(args) {
var len = args.length,
arr = new Array(len), i;
for (i = 0; i < len; i += 1) {
arr[i] = args[i];
}
return arr;
}
module.exports = {
reply_to_strings: replyToStrings,
reply_to_object: replyToObject,
to_array: toArray
};

2
package.json
View File

@@ -20,7 +20,7 @@
"devDependencies": {
"coveralls": "^2.11.2",
"jshint": "^2.8.0",
"metrics": ">=0.1.5",
"metrics": "^0.1.9",
"mocha": "^2.3.2",
"nyc": "^3.2.2",
"optional-dev-dependency": "^1.1.0",

34
test/commands/geoadd.spec.js.future Normal file
View File

@@ -0,0 +1,34 @@
'use strict';
var config = require("../lib/config");
var helper = require("../helper");
var redis = config.redis;
describe("The 'getoadd' method", function () {
helper.allTests(function(parser, ip, args) {
describe("using " + parser + " and " + ip, function () {
var client;
beforeEach(function (done) {
client = redis.createClient.apply(redis.createClient, args);
client.once("connect", function () {
client.flushdb(done);
});
});
it('returns 1 if the key exists', function (done) {
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
done();
});
});
afterEach(function () {
client.end();
});
});
});
});

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

@@ -88,6 +88,16 @@ describe("The 'set' method", function () {
});
}, 100);
});
it("sets the value correctly with the array syntax", function (done) {
client.set([key, value]);
setTimeout(function () {
client.get(key, function (err, res) {
helper.isString(value)(err, res);
done();
});
}, 100);
});
});
describe("with undefined 'key' and missing 'value' parameter", function () {

43
test/commands/sync.spec.js Normal file
View File

@@ -0,0 +1,43 @@
'use strict';
var assert = require('assert');
var config = require("../lib/config");
var helper = require("../helper");
var redis = config.redis;
describe("The 'sync' method", function () {
helper.allTests(function(parser, ip, args) {
describe("using " + parser + " and " + ip, function () {
var client;
beforeEach(function (done) {
client = redis.createClient.apply(redis.createClient, args);
client.once("connect", function () {
client.flushdb(done);
});
});
// This produces a parser error
// Protocol error, got "K" as reply type byte
// I'm uncertain if this is correct behavior or not
it('try to sync with the server and fail other commands', function (done) {
client.on('error', function(err) {
assert.equal(err.message, 'Protocol error, got "K" as reply type byte');
assert.equal(err.command, 'SET');
done();
});
client.sync(function(err, res) {
assert.equal(err, null);
assert(!!res);
});
client.set('foo', 'bar');
});
afterEach(function () {
client.end();
});
});
});
});

6
test/helper.js
View File

@@ -31,12 +31,10 @@ module.exports = {
redisProcess: function () {
return rp;
},
stopRedis: function (done) {
stopRedis: function(done) {
rp.stop(done);
},
startRedis: function (conf, done) {
startRedis(conf, done);
},
startRedis: startRedis,
isNumber: function (expected, done) {
return function (err, results) {
assert.strictEqual(null, err, "expected " + expected + ", got error: " + err);

3
test/lib/config.js
View File

@@ -13,7 +13,8 @@ var config = {
},
configureClient: function (parser, ip, opts) {
var args = [];
opts = opts || {};
// Do not manipulate the opts => copy them each time
opts = opts ? JSON.parse(JSON.stringify(opts)) : {};
if (ip.match(/\.sock/)) {
args.push(ip);

49
test/node_redis.spec.js
View File

@@ -156,8 +156,8 @@ describe("The node_redis client", function () {
process.once('uncaughtException', function (err) {
process.on('uncaughtException', mochaListener);
// assert(/ERR Protocol error/.test(err.message));
// assert.equal(err.command, true);
assert(/ERR Protocol error/.test(err.message));
assert.equal(err.command, true);
assert.equal(err.code, 'ERR');
done();
});
@@ -177,25 +177,36 @@ describe("The node_redis client", function () {
describe(".end", function () {
it('used without flush', function(done) {
var err = null;
client.set('foo', 'bar');
client.end();
client.get('foo', function(err, res) {
err = new Error('failed');
});
setTimeout(function() {
done(err);
}, 200);
var end = helper.callFuncAfter(function() {
done(new Error('failed'));
}, 20);
var cb = function(err, res) {
assert.equal(err.message, "SET can't be processed. The connection has already been closed.");
end();
};
for (var i = 0; i < 20; i++) {
if (i === 10) {
client.end();
}
client.set('foo', 'bar', cb);
}
setTimeout(done, 250);
});
it('used with flush set to true', function(done) {
client.set('foo', 'bar');
client.end();
client.get('foo', function(err, res) {
assert.strictEqual(err.command, 'GET');
assert.strictEqual(err.message, "GET can't be processed. The connection has already been closed.");
var end = helper.callFuncAfter(function() {
done();
});
}, 20);
var cb = function(err, res) {
assert(/The connection has already been closed./.test(err.message));
end();
};
for (var i = 0; i < 20; i++) {
if (i === 10) {
client.end(true);
}
client.set('foo', 'bar', cb);
}
});
});
@@ -730,7 +741,7 @@ describe("The node_redis client", function () {
describe('retry_max_delay', function () {
var client;
var args = config.configureClient(parser, ip, {
retry_max_delay: 1
retry_max_delay: 1 // ms
});
it("sets upper bound on how long client waits before reconnecting", function (done) {
@@ -747,7 +758,7 @@ describe("The node_redis client", function () {
} else {
client.end();
var lasted = new Date().getTime() - time;
assert.ok(lasted < 1000);
assert.ok(lasted < 50);
return done();
}
});

2
test/parser/javascript.spec.js
View File

@@ -1,7 +1,7 @@
'use strict';
var assert = require('assert');
var Parser = require("../../lib/parser/javascript").Parser;
var Parser = require("../../lib/parsers/javascript").Parser;
var config = require("../lib/config");
var redis = config.redis;