Browse Source

Rewrite the logic used to handle Redis commands with clustering.

The reason for this change is to support the upcoming redis-cluster since
it has different behaviors compared to the client-side cluster implementation
provided by Predis. For example redis-cluster will not support key tags or
certain operations currently available with our client-side implementation.
Daniele Alessandri 13 years ago
parent
commit
511fe0bbab
55 changed files with 652 additions and 542 deletions
  1. 4 0
      CHANGELOG.md
  2. 4 42
      lib/Predis/Command/AbstractCommand.php
  3. 9 6
      lib/Predis/Command/CommandInterface.php
  4. 0 8
      lib/Predis/Command/ConnectionAuth.php
  5. 0 8
      lib/Predis/Command/ConnectionEcho.php
  6. 0 8
      lib/Predis/Command/ConnectionPing.php
  7. 0 8
      lib/Predis/Command/ConnectionQuit.php
  8. 0 8
      lib/Predis/Command/ConnectionSelect.php
  9. 303 0
      lib/Predis/Command/Hash/CommandHashStrategy.php
  10. 45 0
      lib/Predis/Command/Hash/CommandHashStrategyInterface.php
  11. 0 13
      lib/Predis/Command/KeyDelete.php
  12. 0 8
      lib/Predis/Command/KeyKeys.php
  13. 0 8
      lib/Predis/Command/KeyMove.php
  14. 0 8
      lib/Predis/Command/KeyRandom.php
  15. 0 8
      lib/Predis/Command/KeyRename.php
  16. 0 10
      lib/Predis/Command/ListPopFirstBlocking.php
  17. 0 8
      lib/Predis/Command/ListPopLastPushHead.php
  18. 0 10
      lib/Predis/Command/ListPopLastPushHeadBlocking.php
  19. 0 8
      lib/Predis/Command/PubSubPublish.php
  20. 0 8
      lib/Predis/Command/PubSubSubscribe.php
  21. 0 8
      lib/Predis/Command/PubSubUnsubscribe.php
  22. 0 8
      lib/Predis/Command/ServerBackgroundRewriteAOF.php
  23. 0 8
      lib/Predis/Command/ServerBackgroundSave.php
  24. 0 8
      lib/Predis/Command/ServerClient.php
  25. 0 8
      lib/Predis/Command/ServerConfig.php
  26. 0 8
      lib/Predis/Command/ServerDatabaseSize.php
  27. 0 8
      lib/Predis/Command/ServerEval.php
  28. 0 8
      lib/Predis/Command/ServerFlushAll.php
  29. 0 8
      lib/Predis/Command/ServerFlushDatabase.php
  30. 0 8
      lib/Predis/Command/ServerInfo.php
  31. 0 8
      lib/Predis/Command/ServerLastSave.php
  32. 0 8
      lib/Predis/Command/ServerMonitor.php
  33. 0 8
      lib/Predis/Command/ServerObject.php
  34. 0 8
      lib/Predis/Command/ServerSave.php
  35. 0 8
      lib/Predis/Command/ServerScript.php
  36. 0 8
      lib/Predis/Command/ServerShutdown.php
  37. 0 8
      lib/Predis/Command/ServerSlaveOf.php
  38. 0 8
      lib/Predis/Command/ServerSlowlog.php
  39. 0 8
      lib/Predis/Command/SetIntersection.php
  40. 0 8
      lib/Predis/Command/SetIntersectionStore.php
  41. 0 8
      lib/Predis/Command/SetMove.php
  42. 0 8
      lib/Predis/Command/StringGetMultiple.php
  43. 0 15
      lib/Predis/Command/StringSetMultiple.php
  44. 0 8
      lib/Predis/Command/TransactionDiscard.php
  45. 0 8
      lib/Predis/Command/TransactionExec.php
  46. 0 8
      lib/Predis/Command/TransactionMulti.php
  47. 0 8
      lib/Predis/Command/TransactionUnwatch.php
  48. 0 8
      lib/Predis/Command/TransactionWatch.php
  49. 0 12
      lib/Predis/Command/ZSetUnionStore.php
  50. 16 9
      lib/Predis/Connection/PredisCluster.php
  51. 0 20
      lib/Predis/Helpers.php
  52. 5 78
      tests/Predis/Command/CommandTest.php
  53. 265 0
      tests/Predis/Command/Hash/CommandHashStrategyTest.php
  54. 1 1
      tests/Predis/Connection/PredisClusterTest.php
  55. 0 14
      tests/Predis/HelpersTest.php

+ 4 - 0
CHANGELOG.md

@@ -35,6 +35,10 @@ v0.8.0 (201x-xx-xx)
   context to make the execution atomic: if a pipeline fails at a certain point
   context to make the execution atomic: if a pipeline fails at a certain point
   then the whole pipeline is discarded.
   then the whole pipeline is discarded.
 
 
+- The key-hashing mechanism for commands is now handled externally and is no
+  more a competence of each command class. This change is neeeded to support
+  both client-side sharding and Redis cluster.
+
 - `Predis\Options\Option` is now abstract, see `Predis\Option\AbstractOption`.
 - `Predis\Options\Option` is now abstract, see `Predis\Option\AbstractOption`.
 
 
 
 

+ 4 - 42
lib/Predis/Command/AbstractCommand.php

@@ -11,8 +11,6 @@
 
 
 namespace Predis\Command;
 namespace Predis\Command;
 
 
-use Predis\Helpers;
-use Predis\Distribution\HashGeneratorInterface;
 
 
 /**
 /**
  * Base class for Redis commands.
  * Base class for Redis commands.
@@ -76,57 +74,21 @@ abstract class AbstractCommand implements CommandInterface
     }
     }
 
 
     /**
     /**
-     * Checks if the command can return an hash for client-side sharding.
-     *
-     * @return Boolean
-     */
-    protected function canBeHashed()
-    {
-        return isset($this->arguments[0]);
-    }
-
-    /**
-     * Checks if the specified array of keys will generate the same hash.
-     *
-     * @param array $keys Array of keys.
-     * @return Boolean
+     * {@inheritdoc}
      */
      */
-    protected function checkSameHashForKeys(Array $keys)
+    public function setHash($hash)
     {
     {
-        if (($count = count($keys)) === 0) {
-            return false;
-        }
-
-        $currentKey = Helpers::extractKeyTag($keys[0]);
-
-        for ($i = 1; $i < $count; $i++) {
-            $nextKey = Helpers::extractKeyTag($keys[$i]);
-            if ($currentKey !== $nextKey) {
-                return false;
-            }
-            $currentKey = $nextKey;
-        }
-
-        return true;
+        $this->hash = $hash;
     }
     }
 
 
     /**
     /**
      * {@inheritdoc}
      * {@inheritdoc}
      */
      */
-    public function getHash(HashGeneratorInterface $hasher)
+    public function getHash()
     {
     {
         if (isset($this->hash)) {
         if (isset($this->hash)) {
             return $this->hash;
             return $this->hash;
         }
         }
-
-        if ($this->canBeHashed()) {
-            $key = Helpers::extractKeyTag($this->arguments[0]);
-            $this->hash = $hasher->hash($key);
-
-            return $this->hash;
-        }
-
-        return null;
     }
     }
 
 
     /**
     /**

+ 9 - 6
lib/Predis/Command/CommandInterface.php

@@ -11,8 +11,6 @@
 
 
 namespace Predis\Command;
 namespace Predis\Command;
 
 
-use Predis\Distribution\HashGeneratorInterface;
-
 /**
 /**
  * Defines an abstraction representing a Redis command.
  * Defines an abstraction representing a Redis command.
  * @author Daniele Alessandri <suppakilla@gmail.com>
  * @author Daniele Alessandri <suppakilla@gmail.com>
@@ -27,13 +25,18 @@ interface CommandInterface
     public function getId();
     public function getId();
 
 
     /**
     /**
-     * Returns an hash of the command using the provided algorithm against the
-     * key (used to calculate the distribution of keys with client-side sharding).
+     * Set the hash for the command.
+     *
+     * @param int $hash Calculated hash.
+     */
+    public function setHash($hash);
+
+    /**
+     * Returns the hash of the command.
      *
      *
-     * @param HashGeneratorInterface $hasher Distribution algorithm.
      * @return int
      * @return int
      */
      */
-    public function getHash(HashGeneratorInterface $hasher);
+    public function getHash();
 
 
     /**
     /**
      * Sets the arguments of the command.
      * Sets the arguments of the command.

+ 0 - 8
lib/Predis/Command/ConnectionAuth.php

@@ -24,12 +24,4 @@ class ConnectionAuth extends AbstractCommand
     {
     {
         return 'AUTH';
         return 'AUTH';
     }
     }
-
-    /**
-     * {@inheritdoc}
-     */
-    protected function canBeHashed()
-    {
-        return false;
-    }
 }
 }

+ 0 - 8
lib/Predis/Command/ConnectionEcho.php

@@ -24,12 +24,4 @@ class ConnectionEcho extends AbstractCommand
     {
     {
         return 'ECHO';
         return 'ECHO';
     }
     }
-
-    /**
-     * {@inheritdoc}
-     */
-    protected function canBeHashed()
-    {
-        return false;
-    }
 }
 }

+ 0 - 8
lib/Predis/Command/ConnectionPing.php

@@ -25,14 +25,6 @@ class ConnectionPing extends AbstractCommand
         return 'PING';
         return 'PING';
     }
     }
 
 
-    /**
-     * {@inheritdoc}
-     */
-    protected function canBeHashed()
-    {
-        return false;
-    }
-
     /**
     /**
      * {@inheritdoc}
      * {@inheritdoc}
      */
      */

+ 0 - 8
lib/Predis/Command/ConnectionQuit.php

@@ -24,12 +24,4 @@ class ConnectionQuit extends AbstractCommand
     {
     {
         return 'QUIT';
         return 'QUIT';
     }
     }
-
-    /**
-     * {@inheritdoc}
-     */
-    protected function canBeHashed()
-    {
-        return false;
-    }
 }
 }

+ 0 - 8
lib/Predis/Command/ConnectionSelect.php

@@ -24,12 +24,4 @@ class ConnectionSelect extends AbstractCommand
     {
     {
         return 'SELECT';
         return 'SELECT';
     }
     }
-
-    /**
-     * {@inheritdoc}
-     */
-    protected function canBeHashed()
-    {
-        return false;
-    }
 }
 }

+ 303 - 0
lib/Predis/Command/Hash/CommandHashStrategy.php

@@ -0,0 +1,303 @@
+<?php
+
+/*
+ * This file is part of the Predis package.
+ *
+ * (c) Daniele Alessandri <suppakilla@gmail.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Predis\Command\Hash;
+
+use Predis\Command\CommandInterface;
+use Predis\Distribution\HashGeneratorInterface;
+
+/**
+ * Default class used by Predis for client-side sharding to calculate
+ * hashes out of keys of supported commands.
+ *
+ * @author Daniele Alessandri <suppakilla@gmail.com>
+ */
+class CommandHashStrategy implements CommandHashStrategyInterface
+{
+    private $commands;
+
+    /**
+     *
+     */
+    public function __construct()
+    {
+        $this->commands = $this->getDefaultCommands();
+    }
+
+    /**
+     * Returns the default map of supported commands with their handlers.
+     *
+     * @return array
+     */
+    protected function getDefaultCommands()
+    {
+        $keyIsFirstArgument = array($this, 'getKeyFromFirstArgument');
+
+        return array(
+            /* commands operating on the key space */
+            'EXISTS'                => $keyIsFirstArgument,
+            'DEL'                   => array($this, 'getKeyFromAllArguments'),
+            'TYPE'                  => $keyIsFirstArgument,
+            'EXPIRE'                => $keyIsFirstArgument,
+            'EXPIREAT'              => $keyIsFirstArgument,
+            'PERSIST'               => $keyIsFirstArgument,
+            'PEXPIRE'               => $keyIsFirstArgument,
+            'PEXPIREAT'             => $keyIsFirstArgument,
+            'TTL'                   => $keyIsFirstArgument,
+            'PTTL'                  => $keyIsFirstArgument,
+            'SORT'                  => $keyIsFirstArgument, // TODO
+
+            /* commands operating on string values */
+            'APPEND'                => $keyIsFirstArgument,
+            'DECR'                  => $keyIsFirstArgument,
+            'DECRBY'                => $keyIsFirstArgument,
+            'GET'                   => $keyIsFirstArgument,
+            'GETBIT'                => $keyIsFirstArgument,
+            'MGET'                  => array($this, 'getKeyFromAllArguments'),
+            'SET'                   => $keyIsFirstArgument,
+            'GETRANGE'              => $keyIsFirstArgument,
+            'GETSET'                => $keyIsFirstArgument,
+            'INCR'                  => $keyIsFirstArgument,
+            'INCRBY'                => $keyIsFirstArgument,
+            'SETBIT'                => $keyIsFirstArgument,
+            'SETEX'                 => $keyIsFirstArgument,
+            'MSET'                  => array($this, 'getKeyFromInterleavedArguments'),
+            'MSETNX'                => array($this, 'getKeyFromInterleavedArguments'),
+            'SETNX'                 => $keyIsFirstArgument,
+            'SETRANGE'              => $keyIsFirstArgument,
+            'STRLEN'                => $keyIsFirstArgument,
+            'SUBSTR'                => $keyIsFirstArgument,
+
+            /* commands operating on lists */
+            'LINSERT'               => $keyIsFirstArgument,
+            'LINDEX'                => $keyIsFirstArgument,
+            'LLEN'                  => $keyIsFirstArgument,
+            'LPOP'                  => $keyIsFirstArgument,
+            'RPOP'                  => $keyIsFirstArgument,
+            'RPOPLPUSH'             => array($this, 'getKeyFromAllArguments'),
+            'BLPOP'                 => array($this, 'getKeyFromBlockingListCommands'),
+            'BRPOP'                 => array($this, 'getKeyFromBlockingListCommands'),
+            'BRPOPLPUSH'            => array($this, 'getKeyFromBlockingListCommands'),
+            'LPUSH'                 => $keyIsFirstArgument,
+            'LPUSHX'                => $keyIsFirstArgument,
+            'RPUSH'                 => $keyIsFirstArgument,
+            'RPUSHX'                => $keyIsFirstArgument,
+            'LRANGE'                => $keyIsFirstArgument,
+            'LREM'                  => $keyIsFirstArgument,
+            'LSET'                  => $keyIsFirstArgument,
+            'LTRIM'                 => $keyIsFirstArgument,
+
+            /* commands operating on sets */
+            'SADD'                  => $keyIsFirstArgument,
+            'SCARD'                 => $keyIsFirstArgument,
+            'SDIFF'                 => array($this, 'getKeyFromAllArguments'),
+            'SDIFFSTORE'            => array($this, 'getKeyFromAllArguments'),
+            'SINTER'                => array($this, 'getKeyFromAllArguments'),
+            'SINTERSTORE'           => array($this, 'getKeyFromAllArguments'),
+            'SUNION'                => array($this, 'getKeyFromAllArguments'),
+            'SUNIONSTORE'           => array($this, 'getKeyFromAllArguments'),
+            'SISMEMBER'             => $keyIsFirstArgument,
+            'SMEMBERS'              => $keyIsFirstArgument,
+            'SPOP'                  => $keyIsFirstArgument,
+            'SRANDMEMBER'           => $keyIsFirstArgument,
+            'SREM'                  => $keyIsFirstArgument,
+
+            /* commands operating on sorted sets */
+            'ZADD'                  => $keyIsFirstArgument,
+            'ZCARD'                 => $keyIsFirstArgument,
+            'ZCOUNT'                => $keyIsFirstArgument,
+            'ZINCRBY'               => $keyIsFirstArgument,
+            'ZINTERSTORE'           => array($this, 'getKeyFromZsetAggregationCommands'),
+            'ZRANGE'                => $keyIsFirstArgument,
+            'ZRANGEBYSCORE'         => $keyIsFirstArgument,
+            'ZRANK'                 => $keyIsFirstArgument,
+            'ZREM'                  => $keyIsFirstArgument,
+            'ZREMRANGEBYRANK'       => $keyIsFirstArgument,
+            'ZREMRANGEBYSCORE'      => $keyIsFirstArgument,
+            'ZREVRANGE'             => $keyIsFirstArgument,
+            'ZREVRANGEBYSCORE'      => $keyIsFirstArgument,
+            'ZREVRANK'              => $keyIsFirstArgument,
+            'ZSCORE'                => $keyIsFirstArgument,
+            'ZUNIONSTORE'           => array($this, 'getKeyFromZsetAggregationCommands'),
+
+            /* commands operating on hashes */
+            'HDEL'                  => $keyIsFirstArgument,
+            'HEXISTS'               => $keyIsFirstArgument,
+            'HGET'                  => $keyIsFirstArgument,
+            'HGETALL'               => $keyIsFirstArgument,
+            'HMGET'                 => $keyIsFirstArgument,
+            'HINCRBY'               => $keyIsFirstArgument,
+            'HINCRBYFLOAT'          => $keyIsFirstArgument,
+            'HKEYS'                 => $keyIsFirstArgument,
+            'HLEN'                  => $keyIsFirstArgument,
+            'HSET'                  => $keyIsFirstArgument,
+            'HSETNX'                => $keyIsFirstArgument,
+            'HVALS'                 => $keyIsFirstArgument,
+        );
+    }
+
+    /**
+     * Returns the list of IDs for the supported commands.
+     *
+     * @return array
+     */
+    public function getSupportedCommands()
+    {
+        return array_keys($this->commands);
+    }
+
+    /**
+     * Extracts the key from the first argument of a command instance.
+     *
+     * @param CommandInterface $command Command instance.
+     * @return string
+     */
+    protected function getKeyFromFirstArgument(CommandInterface $command)
+    {
+        return $command->getArgument(0);
+    }
+
+    /**
+     * Extracts the key from a command with multiple keys only when all keys
+     * in the arguments array produce the same hash.
+     *
+     * @param CommandInterface $command Command instance.
+     * @return string
+     */
+    protected function getKeyFromAllArguments(CommandInterface $command)
+    {
+        $arguments = $command->getArguments();
+
+        if ($this->checkSameHashForKeys($arguments)) {
+            return $arguments[0];
+        }
+    }
+
+    /**
+     * Extracts the key from a command with multiple keys only when all keys
+     * in the arguments array produce the same hash.
+     *
+     * @param CommandInterface $command Command instance.
+     * @return string
+     */
+    protected function getKeyFromInterleavedArguments(CommandInterface $command)
+    {
+        $arguments = $command->getArguments();
+        $keys = array();
+
+        for ($i = 0; $i < count($arguments); $i += 2) {
+            $keys[] = $arguments[$i];
+        }
+
+        if ($this->checkSameHashForKeys($keys)) {
+            return $arguments[0];
+        }
+    }
+
+    /**
+     * Extracts the key from BLPOP and BRPOP commands.
+     *
+     * @param CommandInterface $command Command instance.
+     * @return string
+     */
+    protected function getKeyFromBlockingListCommands(CommandInterface $command)
+    {
+        $arguments = $command->getArguments();
+
+        if ($this->checkSameHashForKeys(array_slice($arguments, 0, count($arguments) - 1))) {
+            return $arguments[0];
+        }
+    }
+
+    /**
+     * Extracts the key from ZINTERSTORE and ZUNIONSTORE commands.
+     *
+     * @param CommandInterface $command Command instance.
+     * @return string
+     */
+    protected function getKeyFromZsetAggregationCommands(CommandInterface $command)
+    {
+        $arguments = $command->getArguments();
+        $keys = array_merge(array($arguments[0]), array_slice($arguments, 2, $arguments[1]));
+
+        if ($this->checkSameHashForKeys($keys)) {
+            return $arguments[0];
+        }
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function getHash(HashGeneratorInterface $hasher, CommandInterface $command)
+    {
+        if (isset($this->commands[$cmdID = $command->getId()])) {
+            if ($key = call_user_func($this->commands[$cmdID], $command)) {
+                return $this->getKeyHash($hasher, $key);
+            }
+        }
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function getKeyHash(HashGeneratorInterface $hasher, $key)
+    {
+        $key = $this->extractKeyTag($key);
+        $hash = $hasher->hash($key);
+
+        return $hash;
+    }
+
+    /**
+     * Checks if the specified array of keys will generate the same hash.
+     *
+     * @param array $keys Array of keys.
+     * @return Boolean
+     */
+    protected function checkSameHashForKeys(Array $keys)
+    {
+        if (($count = count($keys)) === 0) {
+            return false;
+        }
+
+        $currentKey = $this->extractKeyTag($keys[0]);
+
+        for ($i = 1; $i < $count; $i++) {
+            $nextKey = $this->extractKeyTag($keys[$i]);
+            if ($currentKey !== $nextKey) {
+                return false;
+            }
+            $currentKey = $nextKey;
+        }
+
+        return true;
+    }
+
+    /**
+     * Returns only the hashable part of a key (delimited by "{...}"), or the
+     * whole key if a key tag is not found in the string.
+     *
+     * @param string $key A key.
+     * @return string
+     */
+    protected function extractKeyTag($key)
+    {
+        $start = strpos($key, '{');
+        if ($start !== false) {
+            $end = strpos($key, '}', $start);
+            if ($end !== false) {
+                $key = substr($key, ++$start, $end - $start);
+            }
+        }
+
+        return $key;
+    }
+}

+ 45 - 0
lib/Predis/Command/Hash/CommandHashStrategyInterface.php

@@ -0,0 +1,45 @@
+<?php
+
+/*
+ * This file is part of the Predis package.
+ *
+ * (c) Daniele Alessandri <suppakilla@gmail.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Predis\Command\Hash;
+
+use Predis\Command\CommandInterface;
+use Predis\Distribution\HashGeneratorInterface;
+
+/**
+ * Interface for classes defining the strategy used to calculate an hash
+ * out of keys extracted from supported commands.
+ *
+ * This is mostly useful to support clustering via client-side sharding.
+ *
+ * @author Daniele Alessandri <suppakilla@gmail.com>
+ */
+interface CommandHashStrategyInterface
+{
+    /**
+     * Returns the hash for the given command using the specified algorithm, or null
+     * if the command cannot be hashed.
+     *
+     * @param HashGeneratorInterface $hasher Hash algorithm.
+     * @param CommandInterface $command Command to be hashed.
+     * @return int
+     */
+    public function getHash(HashGeneratorInterface $hasher, CommandInterface $command);
+
+    /**
+     * Returns the hash for the given key using the specified algorithm.
+     *
+     * @param HashGeneratorInterface $hasher Hash algorithm.
+     * @param string $key Key to be hashed.
+     * @return string
+     */
+    public function getKeyHash(HashGeneratorInterface $hasher, $key);
+}

+ 0 - 13
lib/Predis/Command/KeyDelete.php

@@ -42,17 +42,4 @@ class KeyDelete extends AbstractCommand implements PrefixableCommandInterface
     {
     {
         PrefixHelpers::all($this, $prefix);
         PrefixHelpers::all($this, $prefix);
     }
     }
-
-    /**
-     * {@inheritdoc}
-     */
-    protected function canBeHashed()
-    {
-        $args = $this->getArguments();
-        if (count($args) === 1) {
-            return true;
-        }
-
-        return $this->checkSameHashForKeys($args);
-    }
 }
 }

+ 0 - 8
lib/Predis/Command/KeyKeys.php

@@ -24,12 +24,4 @@ class KeyKeys extends PrefixableCommand
     {
     {
         return 'KEYS';
         return 'KEYS';
     }
     }
-
-    /**
-     * {@inheritdoc}
-     */
-    protected function canBeHashed()
-    {
-        return false;
-    }
 }
 }

+ 0 - 8
lib/Predis/Command/KeyMove.php

@@ -25,14 +25,6 @@ class KeyMove extends PrefixableCommand
         return 'MOVE';
         return 'MOVE';
     }
     }
 
 
-    /**
-     * {@inheritdoc}
-     */
-    protected function canBeHashed()
-    {
-        return false;
-    }
-
     /**
     /**
      * {@inheritdoc}
      * {@inheritdoc}
      */
      */

+ 0 - 8
lib/Predis/Command/KeyRandom.php

@@ -25,14 +25,6 @@ class KeyRandom extends AbstractCommand
         return 'RANDOMKEY';
         return 'RANDOMKEY';
     }
     }
 
 
-    /**
-     * {@inheritdoc}
-     */
-    protected function canBeHashed()
-    {
-        return false;
-    }
-
     /**
     /**
      * {@inheritdoc}
      * {@inheritdoc}
      */
      */

+ 0 - 8
lib/Predis/Command/KeyRename.php

@@ -32,12 +32,4 @@ class KeyRename extends AbstractCommand implements PrefixableCommandInterface
     {
     {
         PrefixHelpers::all($this, $prefix);
         PrefixHelpers::all($this, $prefix);
     }
     }
-
-    /**
-     * {@inheritdoc}
-     */
-    protected function canBeHashed()
-    {
-        return false;
-    }
 }
 }

+ 0 - 10
lib/Predis/Command/ListPopFirstBlocking.php

@@ -44,14 +44,4 @@ class ListPopFirstBlocking extends AbstractCommand implements PrefixableCommandI
     {
     {
         PrefixHelpers::skipLast($this, $prefix);
         PrefixHelpers::skipLast($this, $prefix);
     }
     }
-
-    /**
-     * {@inheritdoc}
-     */
-    protected function canBeHashed()
-    {
-        return $this->checkSameHashForKeys(
-            array_slice(($args = $this->getArguments()), 0, count($args) - 1)
-        );
-    }
 }
 }

+ 0 - 8
lib/Predis/Command/ListPopLastPushHead.php

@@ -32,12 +32,4 @@ class ListPopLastPushHead extends AbstractCommand implements PrefixableCommandIn
     {
     {
         PrefixHelpers::all($this, $prefix);
         PrefixHelpers::all($this, $prefix);
     }
     }
-
-    /**
-     * {@inheritdoc}
-     */
-    protected function canBeHashed()
-    {
-        return $this->checkSameHashForKeys($this->getArguments());
-    }
 }
 }

+ 0 - 10
lib/Predis/Command/ListPopLastPushHeadBlocking.php

@@ -32,14 +32,4 @@ class ListPopLastPushHeadBlocking extends AbstractCommand implements PrefixableC
     {
     {
         PrefixHelpers::skipLast($this, $prefix);
         PrefixHelpers::skipLast($this, $prefix);
     }
     }
-
-    /**
-     * {@inheritdoc}
-     */
-    protected function canBeHashed()
-    {
-        return $this->checkSameHashForKeys(
-            array_slice($args = $this->getArguments(), 0, count($args) - 1)
-        );
-    }
 }
 }

+ 0 - 8
lib/Predis/Command/PubSubPublish.php

@@ -24,12 +24,4 @@ class PubSubPublish extends PrefixableCommand
     {
     {
         return 'PUBLISH';
         return 'PUBLISH';
     }
     }
-
-    /**
-     * {@inheritdoc}
-     */
-    protected function canBeHashed()
-    {
-        return false;
-    }
 }
 }

+ 0 - 8
lib/Predis/Command/PubSubSubscribe.php

@@ -42,12 +42,4 @@ class PubSubSubscribe extends AbstractCommand implements PrefixableCommandInterf
     {
     {
         PrefixHelpers::all($this, $prefix);
         PrefixHelpers::all($this, $prefix);
     }
     }
-
-    /**
-     * {@inheritdoc}
-     */
-    protected function canBeHashed()
-    {
-        return false;
-    }
 }
 }

+ 0 - 8
lib/Predis/Command/PubSubUnsubscribe.php

@@ -42,12 +42,4 @@ class PubSubUnsubscribe extends AbstractCommand implements PrefixableCommandInte
     {
     {
         PrefixHelpers::all($this, $prefix);
         PrefixHelpers::all($this, $prefix);
     }
     }
-
-    /**
-     * {@inheritdoc}
-     */
-    protected function canBeHashed()
-    {
-        return false;
-    }
 }
 }

+ 0 - 8
lib/Predis/Command/ServerBackgroundRewriteAOF.php

@@ -25,14 +25,6 @@ class ServerBackgroundRewriteAOF extends AbstractCommand
         return 'BGREWRITEAOF';
         return 'BGREWRITEAOF';
     }
     }
 
 
-    /**
-     * {@inheritdoc}
-     */
-    protected function canBeHashed()
-    {
-        return false;
-    }
-
     /**
     /**
      * {@inheritdoc}
      * {@inheritdoc}
      */
      */

+ 0 - 8
lib/Predis/Command/ServerBackgroundSave.php

@@ -25,14 +25,6 @@ class ServerBackgroundSave extends AbstractCommand
         return 'BGSAVE';
         return 'BGSAVE';
     }
     }
 
 
-    /**
-     * {@inheritdoc}
-     */
-    protected function canBeHashed()
-    {
-        return false;
-    }
-
     /**
     /**
      * {@inheritdoc}
      * {@inheritdoc}
      */
      */

+ 0 - 8
lib/Predis/Command/ServerClient.php

@@ -25,14 +25,6 @@ class ServerClient extends AbstractCommand
         return 'CLIENT';
         return 'CLIENT';
     }
     }
 
 
-    /**
-     * {@inheritdoc}
-     */
-    protected function canBeHashed()
-    {
-        return false;
-    }
-
     /**
     /**
      * {@inheritdoc}
      * {@inheritdoc}
      */
      */

+ 0 - 8
lib/Predis/Command/ServerConfig.php

@@ -29,14 +29,6 @@ class ServerConfig extends AbstractCommand
         return 'CONFIG';
         return 'CONFIG';
     }
     }
 
 
-    /**
-     * {@inheritdoc}
-     */
-    protected function canBeHashed()
-    {
-        return false;
-    }
-
     /**
     /**
      * {@inheritdoc}
      * {@inheritdoc}
      */
      */

+ 0 - 8
lib/Predis/Command/ServerDatabaseSize.php

@@ -24,12 +24,4 @@ class ServerDatabaseSize extends AbstractCommand
     {
     {
         return 'DBSIZE';
         return 'DBSIZE';
     }
     }
-
-    /**
-     * {@inheritdoc}
-     */
-    protected function canBeHashed()
-    {
-        return false;
-    }
 }
 }

+ 0 - 8
lib/Predis/Command/ServerEval.php

@@ -39,14 +39,6 @@ class ServerEval extends AbstractCommand implements PrefixableCommandInterface
         $this->setRawArguments($arguments);
         $this->setRawArguments($arguments);
     }
     }
 
 
-    /**
-     * {@inheritdoc}
-     */
-    protected function canBeHashed()
-    {
-        return false;
-    }
-
     /**
     /**
      * Calculates the SHA1 hash of the body of the script.
      * Calculates the SHA1 hash of the body of the script.
      *
      *

+ 0 - 8
lib/Predis/Command/ServerFlushAll.php

@@ -24,12 +24,4 @@ class ServerFlushAll extends AbstractCommand
     {
     {
         return 'FLUSHALL';
         return 'FLUSHALL';
     }
     }
-
-    /**
-     * {@inheritdoc}
-     */
-    protected function canBeHashed()
-    {
-        return false;
-    }
 }
 }

+ 0 - 8
lib/Predis/Command/ServerFlushDatabase.php

@@ -24,12 +24,4 @@ class ServerFlushDatabase extends AbstractCommand
     {
     {
         return 'FLUSHDB';
         return 'FLUSHDB';
     }
     }
-
-    /**
-     * {@inheritdoc}
-     */
-    protected function canBeHashed()
-    {
-        return false;
-    }
 }
 }

+ 0 - 8
lib/Predis/Command/ServerInfo.php

@@ -25,14 +25,6 @@ class ServerInfo extends AbstractCommand
         return 'INFO';
         return 'INFO';
     }
     }
 
 
-    /**
-     * {@inheritdoc}
-     */
-    protected function canBeHashed()
-    {
-        return false;
-    }
-
     /**
     /**
      * {@inheritdoc}
      * {@inheritdoc}
      */
      */

+ 0 - 8
lib/Predis/Command/ServerLastSave.php

@@ -24,12 +24,4 @@ class ServerLastSave extends AbstractCommand
     {
     {
         return 'LASTSAVE';
         return 'LASTSAVE';
     }
     }
-
-    /**
-     * {@inheritdoc}
-     */
-    protected function canBeHashed()
-    {
-        return false;
-    }
 }
 }

+ 0 - 8
lib/Predis/Command/ServerMonitor.php

@@ -24,12 +24,4 @@ class ServerMonitor extends AbstractCommand
     {
     {
         return 'MONITOR';
         return 'MONITOR';
     }
     }
-
-    /**
-     * {@inheritdoc}
-     */
-    protected function canBeHashed()
-    {
-        return false;
-    }
 }
 }

+ 0 - 8
lib/Predis/Command/ServerObject.php

@@ -26,12 +26,4 @@ class ServerObject extends AbstractCommand
     {
     {
         return 'OBJECT';
         return 'OBJECT';
     }
     }
-
-    /**
-     * {@inheritdoc}
-     */
-    protected function canBeHashed()
-    {
-        return false;
-    }
 }
 }

+ 0 - 8
lib/Predis/Command/ServerSave.php

@@ -24,12 +24,4 @@ class ServerSave extends AbstractCommand
     {
     {
         return 'SAVE';
         return 'SAVE';
     }
     }
-
-    /**
-     * {@inheritdoc}
-     */
-    protected function canBeHashed()
-    {
-        return false;
-    }
 }
 }

+ 0 - 8
lib/Predis/Command/ServerScript.php

@@ -24,12 +24,4 @@ class ServerScript extends AbstractCommand
     {
     {
         return 'SCRIPT';
         return 'SCRIPT';
     }
     }
-
-    /**
-     * {@inheritdoc}
-     */
-    protected function canBeHashed()
-    {
-        return false;
-    }
 }
 }

+ 0 - 8
lib/Predis/Command/ServerShutdown.php

@@ -24,12 +24,4 @@ class ServerShutdown extends AbstractCommand
     {
     {
         return 'SHUTDOWN';
         return 'SHUTDOWN';
     }
     }
-
-    /**
-     * {@inheritdoc}
-     */
-    protected function canBeHashed()
-    {
-        return false;
-    }
 }
 }

+ 0 - 8
lib/Predis/Command/ServerSlaveOf.php

@@ -36,12 +36,4 @@ class ServerSlaveOf extends AbstractCommand
 
 
         return $arguments;
         return $arguments;
     }
     }
-
-    /**
-     * {@inheritdoc}
-     */
-    protected function canBeHashed()
-    {
-        return false;
-    }
 }
 }

+ 0 - 8
lib/Predis/Command/ServerSlowlog.php

@@ -27,14 +27,6 @@ class ServerSlowlog extends AbstractCommand
         return 'SLOWLOG';
         return 'SLOWLOG';
     }
     }
 
 
-    /**
-     * {@inheritdoc}
-     */
-    protected function canBeHashed()
-    {
-        return false;
-    }
-
     /**
     /**
      * {@inheritdoc}
      * {@inheritdoc}
      */
      */

+ 0 - 8
lib/Predis/Command/SetIntersection.php

@@ -42,12 +42,4 @@ class SetIntersection extends AbstractCommand implements PrefixableCommandInterf
     {
     {
         PrefixHelpers::all($this, $prefix);
         PrefixHelpers::all($this, $prefix);
     }
     }
-
-    /**
-     * {@inheritdoc}
-     */
-    protected function canBeHashed()
-    {
-        return $this->checkSameHashForKeys($this->getArguments());
-    }
 }
 }

+ 0 - 8
lib/Predis/Command/SetIntersectionStore.php

@@ -44,12 +44,4 @@ class SetIntersectionStore extends AbstractCommand implements PrefixableCommandI
     {
     {
         PrefixHelpers::all($this, $prefix);
         PrefixHelpers::all($this, $prefix);
     }
     }
-
-    /**
-     * {@inheritdoc}
-     */
-    protected function canBeHashed()
-    {
-        return $this->checkSameHashForKeys($this->getArguments());
-    }
 }
 }

+ 0 - 8
lib/Predis/Command/SetMove.php

@@ -33,14 +33,6 @@ class SetMove extends AbstractCommand implements PrefixableCommandInterface
         PrefixHelpers::skipLast($this, $prefix);
         PrefixHelpers::skipLast($this, $prefix);
     }
     }
 
 
-    /**
-     * {@inheritdoc}
-     */
-    protected function canBeHashed()
-    {
-        return false;
-    }
-
     /**
     /**
      * {@inheritdoc}
      * {@inheritdoc}
      */
      */

+ 0 - 8
lib/Predis/Command/StringGetMultiple.php

@@ -42,12 +42,4 @@ class StringGetMultiple extends AbstractCommand implements PrefixableCommandInte
     {
     {
         PrefixHelpers::all($this, $prefix);
         PrefixHelpers::all($this, $prefix);
     }
     }
-
-    /**
-     * {@inheritdoc}
-     */
-    protected function canBeHashed()
-    {
-        return $this->checkSameHashForKeys($this->getArguments());
-    }
 }
 }

+ 0 - 15
lib/Predis/Command/StringSetMultiple.php

@@ -52,19 +52,4 @@ class StringSetMultiple extends AbstractCommand implements PrefixableCommandInte
     {
     {
         PrefixHelpers::interleaved($this, $prefix);
         PrefixHelpers::interleaved($this, $prefix);
     }
     }
-
-    /**
-     * {@inheritdoc}
-     */
-    protected function canBeHashed()
-    {
-        $args = $this->getArguments();
-        $keys = array();
-
-        for ($i = 0; $i < count($args); $i += 2) {
-            $keys[] = $args[$i];
-        }
-
-        return $this->checkSameHashForKeys($keys);
-    }
 }
 }

+ 0 - 8
lib/Predis/Command/TransactionDiscard.php

@@ -24,12 +24,4 @@ class TransactionDiscard extends AbstractCommand
     {
     {
         return 'DISCARD';
         return 'DISCARD';
     }
     }
-
-    /**
-     * {@inheritdoc}
-     */
-    protected function canBeHashed()
-    {
-        return false;
-    }
 }
 }

+ 0 - 8
lib/Predis/Command/TransactionExec.php

@@ -24,12 +24,4 @@ class TransactionExec extends AbstractCommand
     {
     {
         return 'EXEC';
         return 'EXEC';
     }
     }
-
-    /**
-     * {@inheritdoc}
-     */
-    protected function canBeHashed()
-    {
-        return false;
-    }
 }
 }

+ 0 - 8
lib/Predis/Command/TransactionMulti.php

@@ -24,12 +24,4 @@ class TransactionMulti extends AbstractCommand
     {
     {
         return 'MULTI';
         return 'MULTI';
     }
     }
-
-    /**
-     * {@inheritdoc}
-     */
-    protected function canBeHashed()
-    {
-        return false;
-    }
 }
 }

+ 0 - 8
lib/Predis/Command/TransactionUnwatch.php

@@ -25,14 +25,6 @@ class TransactionUnwatch extends AbstractCommand
         return 'UNWATCH';
         return 'UNWATCH';
     }
     }
 
 
-    /**
-     * {@inheritdoc}
-     */
-    protected function canBeHashed()
-    {
-        return false;
-    }
-
     /**
     /**
      * {@inheritdoc}
      * {@inheritdoc}
      */
      */

+ 0 - 8
lib/Predis/Command/TransactionWatch.php

@@ -45,14 +45,6 @@ class TransactionWatch extends AbstractCommand implements PrefixableCommandInter
         PrefixHelpers::all($this, $prefix);
         PrefixHelpers::all($this, $prefix);
     }
     }
 
 
-    /**
-     * {@inheritdoc}
-     */
-    protected function canBeHashed()
-    {
-        return false;
-    }
-
     /**
     /**
      * {@inheritdoc}
      * {@inheritdoc}
      */
      */

+ 0 - 12
lib/Predis/Command/ZSetUnionStore.php

@@ -89,16 +89,4 @@ class ZSetUnionStore extends PrefixableCommand
 
 
         $this->setRawArguments($arguments);
         $this->setRawArguments($arguments);
     }
     }
-
-    /**
-     * {@inheritdoc}
-     */
-    protected function canBeHashed()
-    {
-        $args = $this->getArguments();
-
-        return $this->checkSameHashForKeys(
-            array_merge(array($args[0]), array_slice($args, 2, $args[1]))
-        );
-    }
 }
 }

+ 16 - 9
lib/Predis/Connection/PredisCluster.php

@@ -12,8 +12,8 @@
 namespace Predis\Connection;
 namespace Predis\Connection;
 
 
 use Predis\Command\CommandInterface;
 use Predis\Command\CommandInterface;
+use Predis\Command\Hash\CommandHashStrategy;
 use Predis\Distribution\DistributionStrategyInterface;
 use Predis\Distribution\DistributionStrategyInterface;
-use Predis\Helpers;
 use Predis\ClientException;
 use Predis\ClientException;
 use Predis\NotSupportedException;
 use Predis\NotSupportedException;
 use Predis\Distribution\HashRing;
 use Predis\Distribution\HashRing;
@@ -29,6 +29,7 @@ class PredisCluster implements ClusterConnectionInterface, \IteratorAggregate, \
 {
 {
     private $pool;
     private $pool;
     private $distributor;
     private $distributor;
+    private $cmdHasher;
 
 
     /**
     /**
      * @param DistributionStrategyInterface $distributor Distribution strategy used by the cluster.
      * @param DistributionStrategyInterface $distributor Distribution strategy used by the cluster.
@@ -36,6 +37,7 @@ class PredisCluster implements ClusterConnectionInterface, \IteratorAggregate, \
     public function __construct(DistributionStrategyInterface $distributor = null)
     public function __construct(DistributionStrategyInterface $distributor = null)
     {
     {
         $this->pool = array();
         $this->pool = array();
+        $this->cmdHasher = new CommandHashStrategy();
         $this->distributor = $distributor ?: new HashRing();
         $this->distributor = $distributor ?: new HashRing();
     }
     }
 
 
@@ -126,14 +128,18 @@ class PredisCluster implements ClusterConnectionInterface, \IteratorAggregate, \
      */
      */
     public function getConnection(CommandInterface $command)
     public function getConnection(CommandInterface $command)
     {
     {
-        $cmdHash = $command->getHash($this->distributor);
+        $hash = $command->getHash();
 
 
-        if (isset($cmdHash)) {
-            return $this->distributor->get($cmdHash);
+        if (isset($hash)) {
+            return $this->distributor->get($hash);
         }
         }
 
 
-        $message = sprintf("Cannot send '%s' commands to a cluster of connections", $command->getId());
-        throw new NotSupportedException($message);
+        if ($hash = $this->cmdHasher->getHash($this->distributor, $command)) {
+            $command->setHash($hash);
+            return $this->distributor->get($hash);
+        }
+
+        throw new NotSupportedException("Cannot send {$command->getId()} to a cluster of connections");
     }
     }
 
 
     /**
     /**
@@ -155,10 +161,10 @@ class PredisCluster implements ClusterConnectionInterface, \IteratorAggregate, \
      */
      */
     public function getConnectionByKey($key)
     public function getConnectionByKey($key)
     {
     {
-        $hashablePart = Helpers::extractKeyTag($key);
-        $keyHash = $this->distributor->hash($hashablePart);
+        $hash = $this->cmdHasher->getKeyHash($this->distributor, $key);
+        $node = $this->distributor->get($hash);
 
 
-        return $this->distributor->get($keyHash);
+        return $node;
     }
     }
 
 
     /**
     /**
@@ -210,6 +216,7 @@ class PredisCluster implements ClusterConnectionInterface, \IteratorAggregate, \
     public function executeCommandOnNodes(CommandInterface $command)
     public function executeCommandOnNodes(CommandInterface $command)
     {
     {
         $replies = array();
         $replies = array();
+
         foreach ($this->pool as $connection) {
         foreach ($this->pool as $connection) {
             $replies[] = $connection->executeCommand($command);
             $replies[] = $connection->executeCommand($command);
         }
         }

+ 0 - 20
lib/Predis/Helpers.php

@@ -91,24 +91,4 @@ class Helpers
 
 
         return $arguments;
         return $arguments;
     }
     }
-
-    /**
-     * Returns only the hashable part of a key (delimited by "{...}"), or the
-     * whole key if a key tag is not found in the string.
-     *
-     * @param string $key A key.
-     * @return string
-     */
-    public static function extractKeyTag($key)
-    {
-        $start = strpos($key, '{');
-        if ($start !== false) {
-            $end = strpos($key, '}', $start);
-            if ($end !== false) {
-                $key = substr($key, ++$start, $end - $start);
-            }
-        }
-
-        return $key;
-    }
 }
 }

+ 5 - 78
tests/Predis/Command/CommandTest.php

@@ -95,91 +95,18 @@ class CommandTest extends StandardTestCase
 
 
     /**
     /**
      * @group disconnected
      * @group disconnected
-     * @protected
      */
      */
-    public function testCheckSameHashForKeys()
+    public function testSetAndGetHash()
     {
     {
-        $command = $this->getMockForAbstractClass('Predis\Command\AbstractCommand');
-
-        $checkSameHashForKeys = new \ReflectionMethod($command, 'checkSameHashForKeys');
-        $checkSameHashForKeys->setAccessible(true);
+        $hash = "key-hash";
 
 
-        $this->assertTrue($checkSameHashForKeys->invoke($command, array('foo', '{foo}:bar')));
-        $this->assertFalse($checkSameHashForKeys->invoke($command, array('foo', '{foo}:bar', 'foo:bar')));
-    }
-
-    /**
-     * @group disconnected
-     * @protected
-     */
-    public function testCanBeHashed()
-    {
         $command = $this->getMockForAbstractClass('Predis\Command\AbstractCommand');
         $command = $this->getMockForAbstractClass('Predis\Command\AbstractCommand');
-
-        $canBeHashed = new \ReflectionMethod($command, 'canBeHashed');
-        $canBeHashed->setAccessible(true);
-
-        $this->assertFalse($canBeHashed->invoke($command));
-
         $command->setRawArguments(array('key'));
         $command->setRawArguments(array('key'));
-        $this->assertTrue($canBeHashed->invoke($command));
-    }
-
-    /**
-     * @group disconnected
-     */
-    public function testDoesNotReturnAnHashByDefault()
-    {
-        $distributor = $this->getMock('Predis\Distribution\HashGeneratorInterface');
-        $distributor->expects($this->never())->method('hash');
 
 
-        $command = $this->getMockForAbstractClass('Predis\Command\AbstractCommand');
-
-        $command->getHash($distributor);
-    }
-
-    /**
-     * @group disconnected
-     */
-    public function testReturnAnHashWhenCanBeHashedAndCachesIt()
-    {
-        $key = 'key';
-        $hash = "$key-hash";
-
-        $distributor = $this->getMock('Predis\Distribution\HashGeneratorInterface');
-        $distributor->expects($this->once())
-                    ->method('hash')
-                    ->with($key)
-                    ->will($this->returnValue($hash));
-
-        $command = $this->getMockForAbstractClass('Predis\Command\AbstractCommand');
-        $command->setRawArguments(array($key));
-
-        $this->assertEquals($hash, $command->getHash($distributor));
-
-        $this->assertEquals($hash, $command->getHash($distributor));
-        $this->assertEquals($hash, $command->getHash($distributor));
-    }
-
-    /**
-     * @group disconnected
-     */
-    public function testExtractsKeyTagsBeforeHashing()
-    {
-        $tag = 'key';
-        $key = "{{$tag}}:ignore";
-        $hash = "$tag-hash";
-
-        $distributor = $this->getMock('Predis\Distribution\HashGeneratorInterface');
-        $distributor->expects($this->once())
-                    ->method('hash')
-                    ->with($tag)
-                    ->will($this->returnValue($hash));
-
-        $command = $this->getMockForAbstractClass('Predis\Command\AbstractCommand');
-        $command->setRawArguments(array($key));
+        $this->assertNull($command->getHash());
 
 
-        $this->assertEquals($hash, $command->getHash($distributor));
+        $command->setHash($hash);
+        $this->assertSame($hash, $command->getHash());
     }
     }
 
 
     /**
     /**

+ 265 - 0
tests/Predis/Command/Hash/CommandHashStrategyTest.php

@@ -0,0 +1,265 @@
+<?php
+
+/*
+ * This file is part of the Predis package.
+ *
+ * (c) Daniele Alessandri <suppakilla@gmail.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Predis\Command\Hash;
+
+use \PHPUnit_Framework_TestCase as StandardTestCase;
+use Predis\Profile\ServerProfile;
+use Predis\Distribution\HashRing;
+
+/**
+ *
+ */
+class CommandHashStrategyTest extends StandardTestCase
+{
+    /**
+     * @group disconnected
+     */
+    public function testSupportsKeyTags()
+    {
+        $expected = -1938594527;
+        $distribution = new HashRing();
+        $hashstrategy = new CommandHashStrategy();
+
+        $this->assertSame($expected, $hashstrategy->getKeyHash($distribution, '{foo}'));
+        $this->assertSame($expected, $hashstrategy->getKeyHash($distribution, '{foo}:bar'));
+        $this->assertSame($expected, $hashstrategy->getKeyHash($distribution, '{foo}:baz'));
+        $this->assertSame($expected, $hashstrategy->getKeyHash($distribution, 'bar:{foo}:bar'));
+
+        $this->assertSame(0, $hashstrategy->getKeyHash($distribution, ''));
+        $this->assertSame(0, $hashstrategy->getKeyHash($distribution, '{}'));
+    }
+
+    /**
+     * @group disconnected
+     */
+    public function testSupportedCommands()
+    {
+        $hashstrategy = new CommandHashStrategy();
+
+        $this->assertSame($this->getExpectedCommands(), $hashstrategy->getSupportedCommands());
+    }
+
+    /**
+     * @group disconnected
+     */
+    public function testReturnsNullOnUnsupportedCommand()
+    {
+        $distribution = new HashRing();
+        $hashstrategy = new CommandHashStrategy();
+        $command = ServerProfile::getDevelopment()->createCommand('ping');
+
+        $this->assertNull($hashstrategy->getHash($distribution, $command));
+    }
+
+    /**
+     * @group disconnected
+     */
+    public function testFirstKeyCommands()
+    {
+        $distribution = new HashRing();
+        $hashstrategy = new CommandHashStrategy();
+        $profile = ServerProfile::getDevelopment();
+        $arguments = array('key');
+
+        foreach ($this->getExpectedCommands('keys-first') as $commandID) {
+            $command = $profile->createCommand($commandID, $arguments);
+            $this->assertNotNull($hashstrategy->getHash($distribution, $command), $commandID);
+        }
+    }
+
+    /**
+     * @group disconnected
+     */
+    public function testAllKeysCommands()
+    {
+        $distribution = new HashRing();
+        $hashstrategy = new CommandHashStrategy();
+        $profile = ServerProfile::getDevelopment();
+        $arguments = array('{key}:1', '{key}:2', '{key}:3', '{key}:4');
+
+        foreach ($this->getExpectedCommands('keys-all') as $commandID) {
+            $command = $profile->createCommand($commandID, $arguments);
+            $this->assertNotNull($hashstrategy->getHash($distribution, $command), $commandID);
+        }
+    }
+
+    /**
+     * @group disconnected
+     */
+    public function testInterleavedKeysCommands()
+    {
+        $distribution = new HashRing();
+        $hashstrategy = new CommandHashStrategy();
+        $profile = ServerProfile::getDevelopment();
+        $arguments = array('{key}:1', 'value1', '{key}:2', 'value2');
+
+        foreach ($this->getExpectedCommands('keys-interleaved') as $commandID) {
+            $command = $profile->createCommand($commandID, $arguments);
+            $this->assertNotNull($hashstrategy->getHash($distribution, $command), $commandID);
+        }
+    }
+
+    /**
+     * @group disconnected
+     */
+    public function testKeysForBlockingListCommands()
+    {
+        $distribution = new HashRing();
+        $hashstrategy = new CommandHashStrategy();
+        $profile = ServerProfile::getDevelopment();
+        $arguments = array('{key}:1', '{key}:2', 10);
+
+        foreach ($this->getExpectedCommands('keys-blockinglist') as $commandID) {
+            $command = $profile->createCommand($commandID, $arguments);
+            $this->assertNotNull($hashstrategy->getHash($distribution, $command), $commandID);
+        }
+    }
+
+    /**
+     * @group disconnected
+     */
+    public function testKeysForZsetAggregationCommands()
+    {
+        $distribution = new HashRing();
+        $hashstrategy = new CommandHashStrategy();
+        $profile = ServerProfile::getDevelopment();
+        $arguments = array('{key}:destination', 2, '{key}:1', '{key}:1', array('aggregate' => 'SUM'));
+
+        foreach ($this->getExpectedCommands('keys-zaggregated') as $commandID) {
+            $command = $profile->createCommand($commandID, $arguments);
+            $this->assertNotNull($hashstrategy->getHash($distribution, $command), $commandID);
+        }
+    }
+
+    // ******************************************************************** //
+    // ---- HELPER METHODS ------------------------------------------------ //
+    // ******************************************************************** //
+
+    /**
+     * Returns the list of expected supported commands.
+     *
+     * @param string $type Optional type of command (based on its keys)
+     * @return array
+     */
+    protected function getExpectedCommands($type = null)
+    {
+        $commands = array(
+            /* commands operating on the key space */
+            'EXISTS'                => 'keys-first',
+            'DEL'                   => 'keys-all',
+            'TYPE'                  => 'keys-first',
+            'EXPIRE'                => 'keys-first',
+            'EXPIREAT'              => 'keys-first',
+            'PERSIST'               => 'keys-first',
+            'PEXPIRE'               => 'keys-first',
+            'PEXPIREAT'             => 'keys-first',
+            'TTL'                   => 'keys-first',
+            'PTTL'                  => 'keys-first',
+            'SORT'                  => 'keys-first', // TODO
+
+            /* commands operating on string values */
+            'APPEND'                => 'keys-first',
+            'DECR'                  => 'keys-first',
+            'DECRBY'                => 'keys-first',
+            'GET'                   => 'keys-first',
+            'GETBIT'                => 'keys-first',
+            'MGET'                  => 'keys-all',
+            'SET'                   => 'keys-first',
+            'GETRANGE'              => 'keys-first',
+            'GETSET'                => 'keys-first',
+            'INCR'                  => 'keys-first',
+            'INCRBY'                => 'keys-first',
+            'SETBIT'                => 'keys-first',
+            'SETEX'                 => 'keys-first',
+            'MSET'                  => 'keys-interleaved',
+            'MSETNX'                => 'keys-interleaved',
+            'SETNX'                 => 'keys-first',
+            'SETRANGE'              => 'keys-first',
+            'STRLEN'                => 'keys-first',
+            'SUBSTR'                => 'keys-first',
+
+            /* commands operating on lists */
+            'LINSERT'               => 'keys-first',
+            'LINDEX'                => 'keys-first',
+            'LLEN'                  => 'keys-first',
+            'LPOP'                  => 'keys-first',
+            'RPOP'                  => 'keys-first',
+            'RPOPLPUSH'             => 'keys-all',
+            'BLPOP'                 => 'keys-blockinglist',
+            'BRPOP'                 => 'keys-blockinglist',
+            'BRPOPLPUSH'            => 'keys-blockinglist',
+            'LPUSH'                 => 'keys-first',
+            'LPUSHX'                => 'keys-first',
+            'RPUSH'                 => 'keys-first',
+            'RPUSHX'                => 'keys-first',
+            'LRANGE'                => 'keys-first',
+            'LREM'                  => 'keys-first',
+            'LSET'                  => 'keys-first',
+            'LTRIM'                 => 'keys-first',
+
+            /* commands operating on sets */
+            'SADD'                  => 'keys-first',
+            'SCARD'                 => 'keys-first',
+            'SDIFF'                 => 'keys-all',
+            'SDIFFSTORE'            => 'keys-all',
+            'SINTER'                => 'keys-all',
+            'SINTERSTORE'           => 'keys-all',
+            'SUNION'                => 'keys-all',
+            'SUNIONSTORE'           => 'keys-all',
+            'SISMEMBER'             => 'keys-first',
+            'SMEMBERS'              => 'keys-first',
+            'SPOP'                  => 'keys-first',
+            'SRANDMEMBER'           => 'keys-first',
+            'SREM'                  => 'keys-first',
+
+            /* commands operating on sorted sets */
+            'ZADD'                  => 'keys-first',
+            'ZCARD'                 => 'keys-first',
+            'ZCOUNT'                => 'keys-first',
+            'ZINCRBY'               => 'keys-first',
+            'ZINTERSTORE'           => 'keys-zaggregated',
+            'ZRANGE'                => 'keys-first',
+            'ZRANGEBYSCORE'         => 'keys-first',
+            'ZRANK'                 => 'keys-first',
+            'ZREM'                  => 'keys-first',
+            'ZREMRANGEBYRANK'       => 'keys-first',
+            'ZREMRANGEBYSCORE'      => 'keys-first',
+            'ZREVRANGE'             => 'keys-first',
+            'ZREVRANGEBYSCORE'      => 'keys-first',
+            'ZREVRANK'              => 'keys-first',
+            'ZSCORE'                => 'keys-first',
+            'ZUNIONSTORE'           => 'keys-zaggregated',
+
+            /* commands operating on hashes */
+            'HDEL'                  => 'keys-first',
+            'HEXISTS'               => 'keys-first',
+            'HGET'                  => 'keys-first',
+            'HGETALL'               => 'keys-first',
+            'HMGET'                 => 'keys-first',
+            'HINCRBY'               => 'keys-first',
+            'HINCRBYFLOAT'          => 'keys-first',
+            'HKEYS'                 => 'keys-first',
+            'HLEN'                  => 'keys-first',
+            'HSET'                  => 'keys-first',
+            'HSETNX'                => 'keys-first',
+            'HVALS'                 => 'keys-first',
+        );
+
+        if (isset($type)) {
+            $commands = array_filter($commands, function($expectedType) use($type) {
+                return $expectedType === $type;
+            });
+        }
+
+        return array_keys($commands);
+    }
+}

+ 1 - 1
tests/Predis/Connection/PredisClusterTest.php

@@ -249,7 +249,7 @@ class PredisClusterTest extends StandardTestCase
     /**
     /**
      * @group disconnected
      * @group disconnected
      * @expectedException Predis\NotSupportedException
      * @expectedException Predis\NotSupportedException
-     * @expectedExceptionMessage Cannot send 'PING' commands to a cluster of connections
+     * @expectedExceptionMessage Cannot send PING to a cluster of connections
      */
      */
     public function testThrowsExceptionOnNonShardableCommand()
     public function testThrowsExceptionOnNonShardableCommand()
     {
     {

+ 0 - 14
tests/Predis/HelpersTest.php

@@ -79,18 +79,4 @@ class HelpersTest extends StandardTestCase
         $arguments = array(new \stdClass());
         $arguments = array(new \stdClass());
         $this->assertSame($arguments, Helpers::filterArrayArguments($arguments));
         $this->assertSame($arguments, Helpers::filterArrayArguments($arguments));
     }
     }
-
-    /**
-     * @group disconnected
-     */
-    public function testExtractKeyTag()
-    {
-        $this->assertEquals('foo:bar', Helpers::extractKeyTag('foo:bar'));
-        $this->assertEquals('foo:', Helpers::extractKeyTag('{foo:}bar'));
-        $this->assertEquals('bar', Helpers::extractKeyTag('foo:{bar}'));
-        $this->assertEquals('foo:bar', Helpers::extractKeyTag('{foo:bar}'));
-        $this->assertEquals('', Helpers::extractKeyTag('foo{}:bar'));
-        $this->assertEquals('', Helpers::extractKeyTag(''));
-        $this->assertEquals('', Helpers::extractKeyTag('{}'));
-    }
 }
 }