fix: scripts may not be loaded correctly in pipeline (#1107)

redis · Apr 21, 2020 · 072d460 · 072d460
1 parent 5593aa8
commit 072d460
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 19 deletions.
diff --git a/lib/pipeline.ts b/lib/pipeline.ts
@@ -253,6 +253,7 @@ Pipeline.prototype.exec = function (callback: CallbackFunction) {
 
   // Check whether scripts exists
   const scripts = [];
+  const addedScriptHashes = {};
   for (let i = 0; i < this._queue.length; ++i) {
     const item = this._queue[i];
     if (this.isCluster && item.isCustomCommand) {
@@ -267,10 +268,11 @@ Pipeline.prototype.exec = function (callback: CallbackFunction) {
       continue;
     }
     const script = this._shaToScript[item.args[0]];
-    if (!script) {
+    if (!script || addedScriptHashes[script.sha]) {
       continue;
     }
     scripts.push(script);
+    addedScriptHashes[script.sha] = true;
   }
 
   const _this = this;
@@ -279,7 +281,10 @@ Pipeline.prototype.exec = function (callback: CallbackFunction) {
   }
 
   return this.redis
-    .script("exists", Array.from(new Set(scripts.map(({ sha }) => sha))))
+    .script(
+      "exists",
+      scripts.map(({ sha }) => sha)
+    )
     .then(function (results) {
       const pending = [];
       for (let i = 0; i < results.length; ++i) {

diff --git a/test/functional/pipeline.ts b/test/functional/pipeline.ts
@@ -267,41 +267,56 @@ describe("pipeline", function () {
     it("should check and load uniq scripts only", function (done) {
       const redis = new Redis();
       redis.defineCommand("test", {
-        numberOfKeys: 1,
-        lua: "return {unpack(KEYS),unpack(ARGV)}",
+        numberOfKeys: 2,
+        lua: "return {KEYS[1],KEYS[2],ARGV[1],ARGV[2]}",
       });
       redis.defineCommand("echo", {
         numberOfKeys: 1,
         lua: "return {KEYS[1],ARGV[1]}",
       });
 
       redis.once("ready", function () {
-        const expectedComands = [
-          "script",
-          "script",
-          "script",
-          "evalsha",
-          "evalsha",
-          "evalsha",
-          "evalsha",
+        const expectedCommands = [
+          ["script", "exists"],
+          ["script", "load", "return {KEYS[1],ARGV[1]}"],
+          ["script", "load", "return {KEYS[1],KEYS[2],ARGV[1],ARGV[2]}"],
+          ["evalsha"],
+          ["evalsha"],
+          ["evalsha"],
+          ["evalsha"],
+          ["evalsha"],
+          ["evalsha"],
+        ];
+        const expectedResults = [
+          [null, ["a", "1"]],
+          [null, ["b", "2"]],
+          [null, ["k1", "k2", "v1", "v2"]],
+          [null, ["k3", "k4", "v3", "v4"]],
+          [null, ["c", "3"]],
+          [null, ["k5", "k6", "v5", "v6"]],
         ];
         redis.monitor(function (err, monitor) {
           monitor.on("monitor", function (_, command) {
-            const name = expectedComands.shift();
-            expect(name).to.eql(command[0]);
-            if (!expectedComands.length) {
+            const expectedCommand = expectedCommands.shift();
+            expectedCommand.forEach((arg, i) => expect(arg).to.eql(command[i]));
+            if (!expectedCommands.length) {
               monitor.disconnect();
               redis.disconnect();
               done();
             }
           });
           const pipe = redis.pipeline();
           pipe
-            .echo("f", "0")
-            .test("a", "1")
+            .echo("a", "1")
             .echo("b", "2")
-            .test("b", "3")
-            .exec();
+            .test("k1", "k2", "v1", "v2")
+            .test("k3", "k4", "v3", "v4")
+            .echo("c", "3")
+            .test("k5", "k6", "v5", "v6")
+            .exec(function (err, results) {
+              expect(err).to.eql(null);
+              expect(results).to.eql(expectedResults);
+            });
         });
       });
     });