Switch to string tokens as described in the Redis SET documentation.

Author: willemstuursma

classes/mutex/PHPRedisMutex.php

@@ -36,18 +36,21 @@ public function __construct(array $redisAPIs, string $name, int $timeout = 3)
     {
         parent::__construct($redisAPIs, $name, $timeout);
     }
-    
-    protected function add($redis, string $key, int $value, int $expire): bool
+
+    /**
+     * @throws LockAcquireException
+     */
+    protected function add($redisAPI, string $key, string $value, int $expire): bool
     {
-        /** @var Redis $redis */
+        /** @var Redis $redisAPI */
         try {
             //  Will set the key, if it doesn't exist, with a ttl of $expire seconds
-            return $redis->set($key, $value, ["nx", "ex" => $expire]);
+            return $redisAPI->set($key, $value, ["nx", "ex" => $expire]);
         } catch (RedisException $e) {
             $message = sprintf(
                 "Failed to acquire lock for key '%s' at %s",
                 $key,
-                $this->getRedisIdentifier($redis)
+                $this->getRedisIdentifier($redisAPI)
             );
             throw new LockAcquireException($message, 0, $e);
         }

classes/mutex/PredisMutex.php

@@ -32,17 +32,20 @@ public function __construct(array $clients, string $name, int $timeout = 3)
     {
         parent::__construct($clients, $name, $timeout);
     }
-    
-    protected function add($client, string $key, int $value, int $expire): bool
+
+    /**
+     * @throws LockAcquireException
+     */
+    protected function add($redisAPI, string $key, string $value, int $expire): bool
     {
-        /** @var ClientInterface $client */
+        /** @var ClientInterface $redisAPI */
         try {
-            return $client->set($key, $value, "EX", $expire, "NX") !== null;
+            return $redisAPI->set($key, $value, "EX", $expire, "NX") !== null;
         } catch (PredisException $e) {
             $message = sprintf(
                 "Failed to acquire lock for key '%s' at %s",
                 $key,
-                $this->getRedisIdentifier($client)
+                $this->getRedisIdentifier($redisAPI)
             );
             throw new LockAcquireException($message, 0, $e);
         }
@@ -68,6 +71,14 @@ protected function evalScript($client, string $script, int $numkeys, array $argu
     protected function getRedisIdentifier($client)
     {
         /** @var ClientInterface $client */
+
+        /*
+         * Note that there is no __toString() on the \Predis\Connection\ConnectionInterface
+         * class. Some classes such as \Predis\Connection\Aggregate\SentinelReplication don't
+         * have a __toString() method.
+         *
+         * todo fix this
+         */
         return (string) $client->getConnection();
     }
 }

classes/mutex/RedisMutex.php

@@ -67,9 +67,6 @@ public function setLogger(LoggerInterface $logger)
         $this->logger = $logger;
     }
 
-    /**
-     * @SuppressWarnings(PHPMD)
-     */
     protected function acquire(string $key, int $expire): bool
     {
         // 1. This differs from the specification to avoid an overflow on 32-Bit systems.
@@ -78,7 +75,7 @@ protected function acquire(string $key, int $expire): bool
         // 2.
         $acquired = 0;
         $errored  = 0;
-        $this->token = \random_int(0, 2147483647);
+        $this->token = \random_bytes(16);
         $exception   = null;
         foreach ($this->redisAPIs as $redis) {
             try {
@@ -168,20 +165,18 @@ private function isMajority(int $count): bool
     {
         return $count > count($this->redisAPIs) / 2;
     }
-    
+
     /**
      * Sets the key only if such key doesn't exist at the server yet.
      *
-     * @param mixed  $redisAPI The connected Redis API.
+     * @param mixed $redisAPI The connected Redis API.
      * @param string $key The key.
      * @param string $value The value.
-     * @param int    $expire The TTL seconds.
+     * @param int $expire The TTL seconds.
      *
      * @return bool True, if the key was set.
-     * @throws LockAcquireException An unexpected error happened.
-     * @internal
      */
-    abstract protected function add($redisAPI, string $key, int $value, int $expire): bool;
+    abstract protected function add($redisAPI, string $key, string $value, int $expire): bool;
 
     /**
      * @param mixed  $redisAPI The connected Redis API.
@@ -191,7 +186,6 @@ abstract protected function add($redisAPI, string $key, int $value, int $expire)
      *
      * @return mixed The script result, or false if executing failed.
      * @throws LockReleaseException An unexpected error happened.
-     * @internal
      */
     abstract protected function evalScript($redisAPI, string $script, int $numkeys, array $arguments);
 

composer.json

@@ -63,7 +63,8 @@
     },
     "suggest": {
         "ext-pnctl": "Enables locking with flock without busy waiting in CLI scripts.",
-        "ext-sysvsem": "Enables locking using semaphores",
+        "ext-sysvsem": "Enables locking using semaphores.",
+        "ext-redis": "To use this library with the PHP Redis extension.",
         "predis/predis": "To use this library with predis."
     },
     "archive": {

tests/mutex/PredisMutexTest.php

@@ -47,7 +47,7 @@ public function testAddFailsToSetKey()
     {
         $this->client->expects($this->atLeastOnce())
             ->method("set")
-            ->with("lock_test", $this->isType("integer"), "EX", 4, "NX")
+            ->with("lock_test", $this->isType("string"), "EX", 4, "NX")
             ->willReturn(null);
 
         $this->mutex->synchronized(
@@ -66,7 +66,7 @@ public function testAddErrors()
     {
         $this->client->expects($this->atLeastOnce())
             ->method("set")
-            ->with("lock_test", $this->isType("integer"), "EX", 4, "NX")
+            ->with("lock_test", $this->isType("string"), "EX", 4, "NX")
             ->willThrowException($this->createMock(PredisException::class));
 
         $this->mutex->synchronized(
@@ -80,12 +80,12 @@ public function testWorksNormally()
     {
         $this->client->expects($this->atLeastOnce())
             ->method("set")
-            ->with("lock_test", $this->isType("integer"), "EX", 4, "NX")
+            ->with("lock_test", $this->isType("string"), "EX", 4, "NX")
             ->willReturnSelf();
 
         $this->client->expects($this->once())
             ->method("eval")
-            ->with($this->anything(), 1, "lock_test", $this->isType("integer"))
+            ->with($this->anything(), 1, "lock_test", $this->isType("string"))
             ->willReturn(true);
 
         $executed = false;
@@ -106,12 +106,12 @@ public function testEvalScriptFails()
     {
         $this->client->expects($this->atLeastOnce())
             ->method("set")
-            ->with("lock_test", $this->isType("integer"), "EX", 4, "NX")
+            ->with("lock_test", $this->isType("string"), "EX", 4, "NX")
             ->willReturnSelf();
 
         $this->client->expects($this->once())
             ->method("eval")
-            ->with($this->anything(), 1, "lock_test", $this->isType("integer"))
+            ->with($this->anything(), 1, "lock_test", $this->isType("string"))
             ->willThrowException($this->createMock(PredisException::class));
 
         $executed = false;