1
0
mirror of https://github.com/redis/node-redis.git synced 2025-08-06 02:15:48 +03:00

Remove bad .eval implementation

The implementation is not really good as mentioned in #722 and we pipline our commands.
That way we can't just replace the eval function as it was. This could result in violating the order of execution!

If we want to include a function like this we should not break the order of execution and also not recalculate the sha1 hash each time.
This commit is contained in:
Ruben Bridgewater
2015-09-17 22:51:42 +02:00
parent 13635c9c8c
commit 8b7d4a8449
2 changed files with 21 additions and 42 deletions

View File

@@ -6,7 +6,6 @@ var net = require("net"),
Queue = require("./lib/queue"),
to_array = require("./lib/to_array"),
events = require("events"),
crypto = require("crypto"),
parsers = [],
// This static list of commands is updated from time to time.
// ./lib/commands.js can be updated with generate_commands.js
@@ -1083,37 +1082,6 @@ RedisClient.prototype.multi = RedisClient.prototype.MULTI = function (args) {
return new Multi(this, args);
};
// stash original eval method
var eval_orig = RedisClient.prototype.eval;
// hook eval with an attempt to evalsha for cached scripts
RedisClient.prototype.eval = RedisClient.prototype.EVAL = function () {
var self = this,
args = to_array(arguments),
callback;
if (typeof args[args.length - 1] === "function") {
callback = args.pop();
}
if (Array.isArray(args[0])) {
args = args[0];
}
// replace script source with sha value
var source = args[0];
args[0] = crypto.createHash("sha1").update(source).digest("hex");
self.evalsha(args, function (err, reply) {
if (err && /NOSCRIPT/.test(err.message)) {
args[0] = source;
eval_orig.call(self, args, callback);
} else if (callback) {
callback(err, reply);
}
});
};
var createClient_unix = function(path, options){
var cnxOptions = {
path: path

View File

@@ -12,6 +12,7 @@ describe("The 'eval' method", function () {
describe("using " + parser + " and " + ip, function () {
var client;
var source = "return redis.call('set', 'sha', 'test')";
beforeEach(function (done) {
client = redis.createClient.apply(redis.createClient, args);
@@ -94,24 +95,25 @@ describe("The 'eval' method", function () {
});
});
describe('evalsha', function () {
var source = "return redis.call('get', 'sha test')";
var sha = crypto.createHash('sha1').update(source).digest('hex');
it('allows a script to be executed that accesses the redis API without callback', function (done) {
helper.serverVersionAtLeast.call(this, client, [2, 5, 0]);
client.eval(source, 0);
client.get('sha', helper.isString('test', done));
});
beforeEach(function (done) {
client.set("sha test", "eval get sha test", function (err, res) {
return done(err);
});
});
describe('evalsha', function () {
var sha = crypto.createHash('sha1').update(source).digest('hex');
it('allows a script to be executed that accesses the redis API', function (done) {
helper.serverVersionAtLeast.call(this, client, [2, 5, 0]);
client.eval(source, 0, helper.isString('eval get sha test', done));
client.eval(source, 0, helper.isString('OK'));
client.get('sha', helper.isString('test', done));
});
it('can execute a script if the SHA exists', function (done) {
helper.serverVersionAtLeast.call(this, client, [2, 5, 0]);
client.evalsha(sha, 0, helper.isString('eval get sha test', done));
client.evalsha(sha, 0, helper.isString('OK'));
client.get('sha', helper.isString('test', done));
});
it('returns an error if SHA does not exist', function (done) {
@@ -119,6 +121,15 @@ describe("The 'eval' method", function () {
client.evalsha('ffffffffffffffffffffffffffffffffffffffff', 0, helper.isError(done));
});
it('emit an error if SHA does not exist without any callback', function (done) {
helper.serverVersionAtLeast.call(this, client, [2, 5, 0]);
client.evalsha('ffffffffffffffffffffffffffffffffffffffff', 0);
client.on('error', function(err) {
assert(/NOSCRIPT No matching script. Please use EVAL./.test(err.message));
done();
});
});
it('emits an error if SHA does not exist and no callback has been provided', function (done) {
client.on('error', function (err) {
assert.equal(err.message, 'NOSCRIPT No matching script. Please use EVAL.');