Refactor BRPOPLPUSH and add B[LR]POP documentation

8 files changed, 106 insertions, 51 deletions

diff --git a/redis.stub.php b/redis.stub.php
index 37f3bc8d..c9a95ffd 100644
--- a/redis.stub.php
+++ b/redis.stub.php
@@ -161,7 +161,7 @@ class Redis {
     /**
      * Count the number of set bits in a Redis string.
      *
-     * @see https://https://redis.io/commands/bitcount/
+     * @see https://redis.io/commands/bitcount/
      *
      * @param string $key     The key in question (must be a string key)
      * @param int    $start   The index where Redis should start counting.  If ommitted it
@@ -182,7 +182,7 @@ class Redis {
     /**
      * Return the position of the first bit set to 0 or 1 in a string.
      *
-     * @see https://https://redis.io/commands/bitpos/
+     * @see https://redis.io/commands/bitpos/
      *
      * @param string $key   The key to check (must be a string)
      * @param bool   $bit   Whether to look for an unset (0) or set (1) bit.
@@ -195,11 +195,54 @@ class Redis {
      **/
     public function bitpos(string $key, bool $bit, int $start = 0, int $end = -1, bool $bybit = false): Redis|int|false;
 
-    public function blPop(string|array $key, string|float|int $timeout_or_key, mixed ...$extra_args): Redis|array|null|false;
+    /**
+     * Pop an element off the beginning of a Redis list or lists, potentially blocking up to a specified
+     * timeout.  This method may be called in two distinct ways, of which examples are provided below.
+     *
+     * @see https://redis.io/commands/blpop/
+     *
+     * @param string|array     $key_or_keys    This can either be a string key or an array of one or more
+     *                                         keys.
+     * @param string|float|int $timeout_or_key If the previous argument was a string key, this can either
+     *                                         be an additional key, or the timeout you wish to send to
+     *                                         the command.
+     *
+     * <code>
+     * <?php>
+     * // One way to call this method is in a variadic way, with the final argument being
+     * // the intended timeout.
+     * $redis->blPop('list1', 'list2', 'list3', 1.5);
+     *
+     * // Alternatively, you can send an array of keys
+     * $relay->blPop(['list1', 'list2', 'list3'], 1.5);
+     * ?>
+     * </code>
+     */
+    public function blPop(string|array $key_or_keys, string|float|int $timeout_or_key, mixed ...$extra_args): Redis|array|null|false;
 
-    public function brPop(string|array $key, string|float|int $timeout_or_key, mixed ...$extra_args): Redis|array|null|false;
+    /**
+     * Pop an element off of the end of a Redis list or lists, potentially blocking up to a specified timeout.
+     * The calling convention is identical to Redis::blPop() so see that documentation for more details.
+     *
+     * @see https://redis.io/commands/brpop/
+     * @see Redis::blPop()
+     *
+     */
+    public function brPop(string|array $key_or_keys, string|float|int $timeout_or_key, mixed ...$extra_args): Redis|array|null|false;
 
-    public function brpoplpush(string $src, string $dst, int $timeout): Redis|string|false;
+    /**
+     * Pop an element from the end of a Redis list, pushing it to the beginning of another Redis list,
+     * optionally blocking up to a specified timeout.
+     *
+     * @see https://redis.io/commands/brpoplpush/
+     *
+     * @param string    $src     The source list
+     * @param string    $dst     The destination list
+     * @param int|float $timeout The number of seconds to wait.  Note that you must be connected
+     *                           to Redis >= 6.0.0 to send a floating point timeout.
+     *
+     */
+    public function brpoplpush(string $src, string $dst, int|float $timeout): Redis|string|false;
 
     public function bzPopMax(string|array $key, string|int $timeout_or_key, mixed ...$extra_args): Redis|array|false;
 
@@ -755,7 +798,7 @@ class Redis {
     /**
      * Sort the contents of a Redis key in various ways.
      *
-     * @see https://https://redis.io/commands/sort/
+     * @see https://redis.io/commands/sort/
      *
      * @param string $key     The key you wish to sort
      * @param array  $options Various options controlling how you would like the
@@ -933,7 +976,7 @@ class Redis {
      * How the command works in particular is greatly affected by the options that
      * are passed in.
      *
-     * @see https://https://redis.io/commands/zrange/
+     * @see https://redis.io/commands/zrange/
      * @category zset
      *
      * @param string          $key     The sorted set in question.
@@ -973,7 +1016,7 @@ class Redis {
      * This command is similar to ZRANGE except that instead of returning the values directly
      * it will store them in a destination key provided by the user
      *
-     * @see https://https://redis.io/commands/zrange/
+     * @see https://redis.io/commands/zrange/
      * @see Redis::zRange
      * @category zset
      *
diff --git a/redis_arginfo.h b/redis_arginfo.h
index 4e98b1f5..ba564153 100644
--- a/redis_arginfo.h
+++ b/redis_arginfo.h
@@ -1,5 +1,5 @@
 /* This is a generated file, edit the .stub.php file instead.
- * Stub hash: 21ca57fa960dd8afd88a830b1628e229e831476d */
+ * Stub hash: b42e3cb1c1b50ae6cac12314b83116657365c7c4 */
 
 ZEND_BEGIN_ARG_INFO_EX(arginfo_class_Redis___construct, 0, 0, 0)
 	ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, options, IS_ARRAY, 0, "null")
@@ -72,7 +72,7 @@ ZEND_BEGIN_ARG_WITH_RETURN_OBJ_TYPE_MASK_EX(arginfo_class_Redis_bitpos, 0, 2, Re
 ZEND_END_ARG_INFO()
 
 ZEND_BEGIN_ARG_WITH_RETURN_OBJ_TYPE_MASK_EX(arginfo_class_Redis_blPop, 0, 2, Redis, MAY_BE_ARRAY|MAY_BE_NULL|MAY_BE_FALSE)
-	ZEND_ARG_TYPE_MASK(0, key, MAY_BE_STRING|MAY_BE_ARRAY, NULL)
+	ZEND_ARG_TYPE_MASK(0, key_or_keys, MAY_BE_STRING|MAY_BE_ARRAY, NULL)
 	ZEND_ARG_TYPE_MASK(0, timeout_or_key, MAY_BE_STRING|MAY_BE_DOUBLE|MAY_BE_LONG, NULL)
 	ZEND_ARG_VARIADIC_TYPE_INFO(0, extra_args, IS_MIXED, 0)
 ZEND_END_ARG_INFO()
@@ -82,7 +82,7 @@ ZEND_END_ARG_INFO()
 ZEND_BEGIN_ARG_WITH_RETURN_OBJ_TYPE_MASK_EX(arginfo_class_Redis_brpoplpush, 0, 3, Redis, MAY_BE_STRING|MAY_BE_FALSE)
 	ZEND_ARG_TYPE_INFO(0, src, IS_STRING, 0)
 	ZEND_ARG_TYPE_INFO(0, dst, IS_STRING, 0)
-	ZEND_ARG_TYPE_INFO(0, timeout, IS_LONG, 0)
+	ZEND_ARG_TYPE_MASK(0, timeout, MAY_BE_LONG|MAY_BE_DOUBLE, NULL)
 ZEND_END_ARG_INFO()
 
 ZEND_BEGIN_ARG_WITH_RETURN_OBJ_TYPE_MASK_EX(arginfo_class_Redis_bzPopMax, 0, 2, Redis, MAY_BE_ARRAY|MAY_BE_FALSE)
diff --git a/redis_cluster.stub.php b/redis_cluster.stub.php
index 47cf53bb..de0ddba8 100644
--- a/redis_cluster.stub.php
+++ b/redis_cluster.stub.php
@@ -75,9 +75,19 @@ class RedisCluster {
      */
     public function bitpos(string $key, bool $bit, int $start = 0, int $end = -1, bool $bybit = false): RedisCluster|int|false;
 
+    /**
+     * See Redis::blpop()
+     */
     public function blpop(string|array $key, string|float|int $timeout_or_key, mixed ...$extra_args): RedisCluster|array|null|false;
+
+    /**
+     * See Redis::brpop()
+     */
     public function brpop(string|array $key, string|float|int $timeout_or_key, mixed ...$extra_args): RedisCluster|array|null|false;
 
+    /**
+     * See Redis::brpoplpush()
+     */
     public function brpoplpush(string $srckey, string $deskey, int $timeout): mixed;
 
     public function bzpopmax(string|array $key, string|int $timeout_or_key, mixed ...$extra_args): array;
diff --git a/redis_cluster_arginfo.h b/redis_cluster_arginfo.h
index 9f257cc5..68ae1ffb 100644
--- a/redis_cluster_arginfo.h
+++ b/redis_cluster_arginfo.h
@@ -1,5 +1,5 @@
 /* This is a generated file, edit the .stub.php file instead.
- * Stub hash: aed47186facc916ab9732d986c0fde1b86e2dede */
+ * Stub hash: 3d725e57f5f42243985bca2e64cf727b2475c644 */
 
 ZEND_BEGIN_ARG_INFO_EX(arginfo_class_RedisCluster___construct, 0, 0, 1)
 	ZEND_ARG_TYPE_INFO(0, name, IS_STRING, 1)
diff --git a/redis_cluster_legacy_arginfo.h b/redis_cluster_legacy_arginfo.h
index 3f0c1094..671f10a4 100644
--- a/redis_cluster_legacy_arginfo.h
+++ b/redis_cluster_legacy_arginfo.h
@@ -1,5 +1,5 @@
 /* This is a generated file, edit the .stub.php file instead.
- * Stub hash: aed47186facc916ab9732d986c0fde1b86e2dede */
+ * Stub hash: 3d725e57f5f42243985bca2e64cf727b2475c644 */
 
 ZEND_BEGIN_ARG_INFO_EX(arginfo_class_RedisCluster___construct, 0, 0, 1)
 	ZEND_ARG_INFO(0, name)
diff --git a/redis_commands.c b/redis_commands.c
index d19d6315..56446cab 100644
--- a/redis_commands.c
+++ b/redis_commands.c
@@ -2201,49 +2201,37 @@ redis_getex_cmd(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock,
 int redis_brpoplpush_cmd(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock,
                          char **cmd, int *cmd_len, short *slot, void **ctx)
 {
-    char *key1, *key2;
-    size_t key1_len, key2_len;
-    int key1_free, key2_free;
-    short slot1, slot2;
-    zend_long timeout;
-
-    if (zend_parse_parameters(ZEND_NUM_ARGS(), "ssl", &key1, &key1_len,
-                             &key2, &key2_len, &timeout) == FAILURE)
-    {
-        return FAILURE;
-    }
+    zend_string *src = NULL, *dst = NULL;
+    double timeout = 0;
 
-    // Key prefixing
-    key1_free = redis_key_prefix(redis_sock, &key1, &key1_len);
-    key2_free = redis_key_prefix(redis_sock, &key2, &key2_len);
+    ZEND_PARSE_PARAMETERS_START(3, 3)
+        Z_PARAM_STR(src)
+        Z_PARAM_STR(dst)
+        Z_PARAM_DOUBLE(timeout)
+    ZEND_PARSE_PARAMETERS_END_EX(return FAILURE);
 
-    // In cluster mode, verify the slots match
-    if (slot) {
-        slot1 = cluster_hash_key(key1, key1_len);
-        slot2 = cluster_hash_key(key2, key2_len);
-        if (slot1 != slot2) {
-            php_error_docref(NULL, E_WARNING,
-               "Keys hash to different slots!");
-            if (key1_free) efree(key1);
-            if (key2_free) efree(key2);
-            return FAILURE;
-        }
+    src = redis_key_prefix_zstr(redis_sock, src);
+    dst = redis_key_prefix_zstr(redis_sock, dst);
 
-        // Both slots are the same
-        *slot = slot1;
+    if (slot && (*slot = cluster_hash_key_zstr(src)) != cluster_hash_key_zstr(dst)) {
+        php_error_docref(NULL, E_WARNING, "Keys must hash to the same slot");
+        zend_string_release(src);
+        zend_string_release(dst);
+        return FAILURE;
     }
 
-    // Consistency with Redis, if timeout < 0 use RPOPLPUSH
+    /* Consistency with Redis.  If timeout < 0 use RPOPLPUSH */
     if (timeout < 0) {
-        *cmd_len = REDIS_CMD_SPPRINTF(cmd, "RPOPLPUSH", "ss", key1, key1_len,
-                                     key2, key2_len);
+        *cmd_len = REDIS_CMD_SPPRINTF(cmd, "RPOPLPUSH", "SS", src, dst);
+    } else if (fabs(timeout - (long)timeout) < .0001) {
+        *cmd_len = REDIS_CMD_SPPRINTF(cmd, "BRPOPLPUSH", "SSd", src, dst, (long)timeout);
     } else {
-        *cmd_len = REDIS_CMD_SPPRINTF(cmd, "BRPOPLPUSH", "ssd", key1, key1_len,
-                                     key2, key2_len, timeout);
+        *cmd_len = REDIS_CMD_SPPRINTF(cmd, "BRPOPLPUSH", "SSf", src, dst, timeout);
     }
 
-    if (key1_free) efree(key1);
-    if (key2_free) efree(key2);
+    zend_string_release(src);
+    zend_string_release(dst);
+
     return SUCCESS;
 }
 
diff --git a/redis_legacy_arginfo.h b/redis_legacy_arginfo.h
index adda8981..47a600a7 100644
--- a/redis_legacy_arginfo.h
+++ b/redis_legacy_arginfo.h
@@ -1,5 +1,5 @@
 /* This is a generated file, edit the .stub.php file instead.
- * Stub hash: 21ca57fa960dd8afd88a830b1628e229e831476d */
+ * Stub hash: b42e3cb1c1b50ae6cac12314b83116657365c7c4 */
 
 ZEND_BEGIN_ARG_INFO_EX(arginfo_class_Redis___construct, 0, 0, 0)
 	ZEND_ARG_INFO(0, options)
@@ -67,7 +67,7 @@ ZEND_BEGIN_ARG_INFO_EX(arginfo_class_Redis_bitpos, 0, 0, 2)
 ZEND_END_ARG_INFO()
 
 ZEND_BEGIN_ARG_INFO_EX(arginfo_class_Redis_blPop, 0, 0, 2)
-	ZEND_ARG_INFO(0, key)
+	ZEND_ARG_INFO(0, key_or_keys)
 	ZEND_ARG_INFO(0, timeout_or_key)
 	ZEND_ARG_VARIADIC_INFO(0, extra_args)
 ZEND_END_ARG_INFO()
@@ -80,9 +80,13 @@ ZEND_BEGIN_ARG_INFO_EX(arginfo_class_Redis_brpoplpush, 0, 0, 3)
 	ZEND_ARG_INFO(0, timeout)
 ZEND_END_ARG_INFO()
 
-#define arginfo_class_Redis_bzPopMax arginfo_class_Redis_blPop
+ZEND_BEGIN_ARG_INFO_EX(arginfo_class_Redis_bzPopMax, 0, 0, 2)
+	ZEND_ARG_INFO(0, key)
+	ZEND_ARG_INFO(0, timeout_or_key)
+	ZEND_ARG_VARIADIC_INFO(0, extra_args)
+ZEND_END_ARG_INFO()
 
-#define arginfo_class_Redis_bzPopMin arginfo_class_Redis_blPop
+#define arginfo_class_Redis_bzPopMin arginfo_class_Redis_bzPopMax
 
 ZEND_BEGIN_ARG_INFO_EX(arginfo_class_Redis_bzmpop, 0, 0, 3)
 	ZEND_ARG_INFO(0, timeout)
diff --git a/tests/RedisTest.php b/tests/RedisTest.php
index cd9f631b..466a83a6 100644
--- a/tests/RedisTest.php
+++ b/tests/RedisTest.php
@@ -2498,6 +2498,7 @@ class Redis_Test extends TestSuite
         $this->redis->lpush('{list}y', '456');    // y = [456, 123]
 
         $this->assertEquals($this->redis->brpoplpush('{list}x', '{list}y', 1), 'abc');  // we RPOP x, yielding abc.
+
         $this->assertEquals($this->redis->lrange('{list}x', 0, -1), ['def']); // only def remains in x.
         $this->assertEquals($this->redis->lrange('{list}y', 0, -1), ['abc', '456', '123']);   // abc has been lpushed to y.
 
@@ -2506,6 +2507,15 @@ class Redis_Test extends TestSuite
         $this->assertTrue(FALSE === $this->redis->brpoplpush('{list}x', '{list}y', 1));
         $this->assertTrue([] === $this->redis->lrange('{list}x', 0, -1));
         $this->assertTrue([] === $this->redis->lrange('{list}y', 0, -1));
+
+        if (!$this->minVersionCheck('6.0.0'))
+            return;
+
+        // Redis >= 6.0.0 allows floating point timeouts
+        $st = microtime(true);
+        $this->assertEquals(FALSE, $this->redis->brpoplpush('{list}x', '{list}y', .1));
+        $et = microtime(true);
+        $this->assertTrue($et - $st < 1.0);
     }
 
     public function testZAddFirstArg() {