Browse Source

Use 0 to indicate no arguments for KEYS[] in Lua scripted commands.

We previously used FALSE for that but in the end it does not make much
sense. Luckily for us this does not represent a breaking change since
existing code will keep to work, so we can safely push this change into
the next patch release.
Daniele Alessandri 12 years ago
parent
commit
729e40d6c0
3 changed files with 48 additions and 12 deletions
  1. 4 0
      CHANGELOG.md
  2. 6 12
      lib/Predis/Command/ScriptedCommand.php
  3. 38 0
      tests/Predis/Command/ScriptedCommandTest.php

+ 4 - 0
CHANGELOG.md

@@ -19,6 +19,10 @@ v0.8.3 (2013-xx-xx)
   returning `Predis\Connection\ConnectionInterface`. Users can create their
   own self-contained strategies to create and set up the underlying connection.
 
+- Users should return `0` from `Predis\Command\ScriptedCommand::getKeysCount()`
+  instead of `FALSE` to indicate that all of the arguments of a Lua script must
+  be used to populate `ARGV[]`. This does not represent a breaking change.
+
 
 v0.8.2 (2013-02-03)
 ===============================================================================

+ 6 - 12
lib/Predis/Command/ScriptedCommand.php

@@ -30,19 +30,15 @@ abstract class ScriptedCommand extends ServerEvalSHA
     /**
      * Specifies the number of arguments that should be considered as keys.
      *
-     * The default behaviour for the base class is to return FALSE to indicate that
+     * The default behaviour for the base class is to return 0 to indicate that
      * all the elements of the arguments array should be considered as keys, but
      * subclasses can enforce a static number of keys.
      *
-     * @todo How about returning 1 by default to make scripted commands act like
-     *       variadic ones where the first argument is the key (KEYS[1]) and the
-     *       rest are values (ARGV)?
-     *
-     * @return int|Boolean
+     * @return int
      */
     protected function getKeysCount()
     {
-        return false;
+        return 0;
     }
 
     /**
@@ -60,13 +56,11 @@ abstract class ScriptedCommand extends ServerEvalSHA
      */
     protected function filterArguments(Array $arguments)
     {
-        if (false !== $numkeys = $this->getKeysCount()) {
-            $numkeys = $numkeys >= 0 ? $numkeys : count($arguments) + $numkeys;
-        } else {
-            $numkeys = count($arguments);
+        if (($numkeys = $this->getKeysCount()) && $numkeys < 0) {
+            $numkeys = count($arguments) + $numkeys;
         }
 
-        return array_merge(array(sha1($this->getScript()), $numkeys), $arguments);
+        return array_merge(array(sha1($this->getScript()), (int) $numkeys), $arguments);
     }
 
     /**

+ 38 - 0
tests/Predis/Command/ScriptedCommandTest.php

@@ -60,6 +60,25 @@ class ScriptedCommandTest extends StandardTestCase
         $this->assertSame(array_merge(array(self::LUA_SCRIPT_SHA1, 2), $arguments), $command->getArguments());
     }
 
+    /**
+     * @group disconnected
+     */
+    public function testGetArgumentsWithZeroKeysCount()
+    {
+        $arguments = array('value1', 'value2', 'value3');
+
+        $command = $this->getMock('Predis\Command\ScriptedCommand', array('getScript', 'getKeysCount'));
+        $command->expects($this->once())
+                ->method('getScript')
+                ->will($this->returnValue(self::LUA_SCRIPT));
+        $command->expects($this->once())
+                ->method('getKeysCount')
+                ->will($this->returnValue(0));
+        $command->setArguments($arguments);
+
+        $this->assertSame(array_merge(array(self::LUA_SCRIPT_SHA1, 0), $arguments), $command->getArguments());
+    }
+
     /**
      * @group disconnected
      */
@@ -79,6 +98,25 @@ class ScriptedCommandTest extends StandardTestCase
         $this->assertSame(array('key1', 'key2'), $command->getKeys());
     }
 
+    /**
+     * @group disconnected
+     */
+    public function testGetKeysWithZeroKeysCount()
+    {
+        $arguments = array('value1', 'value2', 'value3');
+
+        $command = $this->getMock('Predis\Command\ScriptedCommand', array('getScript', 'getKeysCount'));
+        $command->expects($this->once())
+                ->method('getScript')
+                ->will($this->returnValue(self::LUA_SCRIPT));
+        $command->expects($this->exactly(2))
+                ->method('getKeysCount')
+                ->will($this->returnValue(0));
+        $command->setArguments($arguments);
+
+        $this->assertSame(array(), $command->getKeys());
+    }
+
     /**
      * @group disconnected
      */