Issue #1894issue-1894-pop-count

Add the COUNT argument to LPOP and RPOP

15 files changed, 109 insertions, 40 deletions

diff --git a/cluster_library.c b/cluster_library.c
index 8ebc8760..976cb889 100644
--- a/cluster_library.c
+++ b/cluster_library.c
@@ -1755,6 +1755,18 @@ PHP_REDIS_API void cluster_ping_resp(INTERNAL_FUNCTION_PARAMETERS, redisCluster
 }
 
 PHP_REDIS_API void
+cluster_pop_resp(INTERNAL_FUNCTION_PARAMETERS, redisCluster *c, void *ctx)
+{
+    if (ctx == NULL) {
+        cluster_bulk_resp(INTERNAL_FUNCTION_PARAM_PASSTHRU, c, NULL);
+    } else if (ctx == PHPREDIS_CTX_PTR) {
+        cluster_mbulk_raw_resp(INTERNAL_FUNCTION_PARAM_PASSTHRU, c, NULL);
+    } else {
+        ZEND_ASSERT(!"memory corruption?");
+    }
+}
+
+PHP_REDIS_API void
 cluster_set_resp(INTERNAL_FUNCTION_PARAMETERS, redisCluster *c, void *ctx)
 {
     if (ctx == NULL) {
diff --git a/cluster_library.h b/cluster_library.h
index 811c435d..995e244f 100644
--- a/cluster_library.h
+++ b/cluster_library.h
@@ -413,6 +413,8 @@ PHP_REDIS_API void cluster_bool_resp(INTERNAL_FUNCTION_PARAMETERS, redisCluster
     void *ctx);
 PHP_REDIS_API void cluster_ping_resp(INTERNAL_FUNCTION_PARAMETERS, redisCluster *c,
     void *ctx);
+PHP_REDIS_API void cluster_pop_resp(INTERNAL_FUNCTION_PARAMETERS, redisCluster *c,
+    void *ctx);
 PHP_REDIS_API void cluster_set_resp(INTERNAL_FUNCTION_PARAMETERS, redisCluster *c,
     void *ctx);
 PHP_REDIS_API void cluster_single_line_resp(INTERNAL_FUNCTION_PARAMETERS, redisCluster *c,
diff --git a/library.c b/library.c
index 6c2b24de..1bf3c3b4 100644
--- a/library.c
+++ b/library.c
@@ -1338,6 +1338,18 @@ redis_hrandfield_response(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock, z
 }
 
 PHP_REDIS_API int
+redis_pop_response(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock, zval *z_tab, void *ctx)
+{
+    if (ctx == NULL) {
+        return redis_string_response(INTERNAL_FUNCTION_PARAM_PASSTHRU, redis_sock, z_tab, NULL);
+    } else if (ctx == PHPREDIS_CTX_PTR) {
+        return redis_mbulk_reply_raw(INTERNAL_FUNCTION_PARAM_PASSTHRU, redis_sock, z_tab, NULL);
+    } else {
+        ZEND_ASSERT(!"memory corruption?");
+    }
+}
+
+PHP_REDIS_API int
 redis_boolean_response_impl(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock,
                             zval *z_tab, void *ctx,
                             SuccessCallback success_callback)
diff --git a/library.h b/library.h
index cbf26a73..e00e6939 100644
--- a/library.h
+++ b/library.h
@@ -167,6 +167,7 @@ PHP_REDIS_API int redis_zdiff_response(INTERNAL_FUNCTION_PARAMETERS, RedisSock *
 PHP_REDIS_API int redis_set_response(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock, zval *z_tab, void *ctx);
 PHP_REDIS_API int redis_geosearch_response(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock, zval *z_tab, void *ctx);
 PHP_REDIS_API int redis_hrandfield_response(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock, zval *z_tab, void *ctx);
+PHP_REDIS_API int redis_pop_response(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock, zval *z_tab, void *ctx);
 
 /* Helper methods to get configuration values from a HashTable. */
 
diff --git a/redis.c b/redis.c
index d526b99d..bfa72b88 100644
--- a/redis.c
+++ b/redis.c
@@ -1280,17 +1280,17 @@ PHP_METHOD(Redis, rPushx)
 }
 /* }}} */
 
-/* {{{ proto string Redis::lPOP(string key) */
+/* {{{ proto string Redis::lPop(string key, [int count = 0]) */
 PHP_METHOD(Redis, lPop)
 {
-    REDIS_PROCESS_KW_CMD("LPOP", redis_key_cmd, redis_string_response);
+    REDIS_PROCESS_KW_CMD("LPOP", redis_pop_cmd, redis_pop_response);
 }
 /* }}} */
 
-/* {{{ proto string Redis::rPOP(string key) */
+/* {{{ proto string Redis::rPop(string key, [int count = 0]) */
 PHP_METHOD(Redis, rPop)
 {
-    REDIS_PROCESS_KW_CMD("RPOP", redis_key_cmd, redis_string_response);
+    REDIS_PROCESS_KW_CMD("RPOP", redis_pop_cmd, redis_pop_response);
 }
 /* }}} */
 
diff --git a/redis.stub.php b/redis.stub.php
index 5245f490..f0f8a37a 100644
--- a/redis.stub.php
+++ b/redis.stub.php
@@ -238,8 +238,7 @@ class Redis {
 
     public function lMove(string $src, string $dst, string $wherefrom, string $whereto): string;
 
-	/** @return string|Redis */
-    public function lPop(string $key);
+    public function lPop(string $key, int $count = 0): bool|string|array;
 
     /**
      * @param mixed $elements
@@ -330,8 +329,7 @@ public function persist(string $key): bool;
 
     public function punsubscribe(array $patterns): array;
 
-	/** @return string|Redis */
-    public function rPop(string $key);
+    public function rPop(string $key, int $count = 0): bool|string|array;
 
 	/** @return string|Redis */
     public function randomKey();
diff --git a/redis_arginfo.h b/redis_arginfo.h
index e615c6de..ee479548 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: 858f814d5b91c0829ae6b6a265a740cc037586dd */
+ * Stub hash: 9671c30926e8d581a126833360b123c8ae2dd913 */
 
 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")
@@ -411,7 +411,10 @@ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_class_Redis_lMove, 0, 4, IS_STRI
 	ZEND_ARG_TYPE_INFO(0, whereto, IS_STRING, 0)
 ZEND_END_ARG_INFO()
 
-#define arginfo_class_Redis_lPop arginfo_class_Redis_decr
+ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_class_Redis_lPop, 0, 1, MAY_BE_BOOL|MAY_BE_STRING|MAY_BE_ARRAY)
+	ZEND_ARG_TYPE_INFO(0, key, IS_STRING, 0)
+	ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, count, IS_LONG, 0, "0")
+ZEND_END_ARG_INFO()
 
 ZEND_BEGIN_ARG_INFO_EX(arginfo_class_Redis_lPush, 0, 0, 1)
 	ZEND_ARG_TYPE_INFO(0, key, IS_STRING, 0)
@@ -558,7 +561,7 @@ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_class_Redis_punsubscribe, 0, 1,
 	ZEND_ARG_TYPE_INFO(0, patterns, IS_ARRAY, 0)
 ZEND_END_ARG_INFO()
 
-#define arginfo_class_Redis_rPop arginfo_class_Redis_decr
+#define arginfo_class_Redis_rPop arginfo_class_Redis_lPop
 
 #define arginfo_class_Redis_randomKey arginfo_class_Redis___destruct
 
diff --git a/redis_cluster.c b/redis_cluster.c
index 140f51bc..5634560f 100644
--- a/redis_cluster.c
+++ b/redis_cluster.c
@@ -820,15 +820,15 @@ PHP_METHOD(RedisCluster, type) {
 }
 /* }}} */
 
-/* {{{ proto string RedisCluster::pop(string key) */
+/* {{{ proto string RedisCluster::pop(string key, [int count = 0]) */
 PHP_METHOD(RedisCluster, lpop) {
-    CLUSTER_PROCESS_KW_CMD("LPOP", redis_key_cmd, cluster_bulk_resp, 0);
+    CLUSTER_PROCESS_KW_CMD("LPOP", redis_pop_cmd, cluster_pop_resp, 0);
 }
 /* }}} */
 
-/* {{{ proto string RedisCluster::rpop(string key) */
+/* {{{ proto string RedisCluster::rpop(string key, [int count = 0]) */
 PHP_METHOD(RedisCluster, rpop) {
-    CLUSTER_PROCESS_KW_CMD("RPOP", redis_key_cmd, cluster_bulk_resp, 0);
+    CLUSTER_PROCESS_KW_CMD("RPOP", redis_pop_cmd, cluster_pop_resp, 0);
 }
 /* }}} */
 
diff --git a/redis_cluster.stub.php b/redis_cluster.stub.php
index c48c4192..2fe512fd 100644
--- a/redis_cluster.stub.php
+++ b/redis_cluster.stub.php
@@ -175,7 +175,7 @@ class RedisCluster {
 
     public function llen(string $key): int|bool;
 
-    public function lpop(string $key): string|bool;
+    public function lpop(string $key, int $count = 0): bool|string|array;
 
     public function lpush(string $key, mixed $value, mixed ...$other_values): int|bool;
 
@@ -237,7 +237,7 @@ class RedisCluster {
 
     public function role(string|array $key_or_address): mixed;
 
-    public function rpop(string $key): bool|string;
+    public function rpop(string $key, int $count = 0): bool|string|array;
 
     public function rpoplpush(string $src, string $dst): bool|string;
 
diff --git a/redis_cluster_arginfo.h b/redis_cluster_arginfo.h
index eda09a45..95945fc3 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: 7f045c90abd99a53f8fb6557942cd17e00ee8a01 */
+ * Stub hash: 8029a0d6df2bbd9cf5d140ff8d9efcc4de2a5bcc */
 
 ZEND_BEGIN_ARG_INFO_EX(arginfo_class_RedisCluster___construct, 0, 0, 1)
 	ZEND_ARG_TYPE_INFO(0, name, IS_STRING, 1)
@@ -367,8 +367,9 @@ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_class_RedisCluster_llen, 0, 1, M
 	ZEND_ARG_TYPE_INFO(0, key, IS_STRING, 0)
 ZEND_END_ARG_INFO()
 
-ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_class_RedisCluster_lpop, 0, 1, MAY_BE_STRING|MAY_BE_BOOL)
+ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_class_RedisCluster_lpop, 0, 1, MAY_BE_BOOL|MAY_BE_STRING|MAY_BE_ARRAY)
 	ZEND_ARG_TYPE_INFO(0, key, IS_STRING, 0)
+	ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, count, IS_LONG, 0, "0")
 ZEND_END_ARG_INFO()
 
 ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_class_RedisCluster_lpush, 0, 2, MAY_BE_LONG|MAY_BE_BOOL)
@@ -502,7 +503,7 @@ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_class_RedisCluster_role, 0, 1, I
 	ZEND_ARG_TYPE_MASK(0, key_or_address, MAY_BE_STRING|MAY_BE_ARRAY, NULL)
 ZEND_END_ARG_INFO()
 
-#define arginfo_class_RedisCluster_rpop arginfo_class_RedisCluster__prefix
+#define arginfo_class_RedisCluster_rpop arginfo_class_RedisCluster_lpop
 
 ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_class_RedisCluster_rpoplpush, 0, 2, MAY_BE_BOOL|MAY_BE_STRING)
 	ZEND_ARG_TYPE_INFO(0, src, IS_STRING, 0)
diff --git a/redis_cluster_legacy_arginfo.h b/redis_cluster_legacy_arginfo.h
index 38a80896..bc07b117 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: 7f045c90abd99a53f8fb6557942cd17e00ee8a01 */
+ * Stub hash: 8029a0d6df2bbd9cf5d140ff8d9efcc4de2a5bcc */
 
 ZEND_BEGIN_ARG_INFO_EX(arginfo_class_RedisCluster___construct, 0, 0, 1)
 	ZEND_ARG_INFO(0, name)
@@ -320,7 +320,10 @@ ZEND_END_ARG_INFO()
 
 #define arginfo_class_RedisCluster_llen arginfo_class_RedisCluster__prefix
 
-#define arginfo_class_RedisCluster_lpop arginfo_class_RedisCluster__prefix
+ZEND_BEGIN_ARG_INFO_EX(arginfo_class_RedisCluster_lpop, 0, 0, 1)
+	ZEND_ARG_INFO(0, key)
+	ZEND_ARG_INFO(0, count)
+ZEND_END_ARG_INFO()
 
 ZEND_BEGIN_ARG_INFO_EX(arginfo_class_RedisCluster_lpush, 0, 0, 2)
 	ZEND_ARG_INFO(0, key)
@@ -433,7 +436,7 @@ ZEND_END_ARG_INFO()
 
 #define arginfo_class_RedisCluster_role arginfo_class_RedisCluster_bgrewriteaof
 
-#define arginfo_class_RedisCluster_rpop arginfo_class_RedisCluster__prefix
+#define arginfo_class_RedisCluster_rpop arginfo_class_RedisCluster_lpop
 
 ZEND_BEGIN_ARG_INFO_EX(arginfo_class_RedisCluster_rpoplpush, 0, 0, 2)
 	ZEND_ARG_INFO(0, src)
@@ -525,10 +528,7 @@ ZEND_END_ARG_INFO()
 
 #define arginfo_class_RedisCluster_spop arginfo_class_RedisCluster__prefix
 
-ZEND_BEGIN_ARG_INFO_EX(arginfo_class_RedisCluster_srandmember, 0, 0, 1)
-	ZEND_ARG_INFO(0, key)
-	ZEND_ARG_INFO(0, count)
-ZEND_END_ARG_INFO()
+#define arginfo_class_RedisCluster_srandmember arginfo_class_RedisCluster_lpop
 
 #define arginfo_class_RedisCluster_srem arginfo_class_RedisCluster_lpush
 
diff --git a/redis_commands.c b/redis_commands.c
index 86ec2539..3b3fa917 100644
--- a/redis_commands.c
+++ b/redis_commands.c
@@ -1601,6 +1601,35 @@ int redis_blocking_pop_cmd(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock,
  * have specific processing (argument validation, etc) that make them unique
  */
 
+int
+redis_pop_cmd(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock,
+              char *kw, char **cmd, int *cmd_len, short *slot, void **ctx)
+{
+    char *key;
+    size_t key_len;
+    smart_string cmdstr = {0};
+    zend_long count = 0;
+
+    // Make sure the function is being called correctly
+    if (zend_parse_parameters(ZEND_NUM_ARGS(), "s|l",
+                              &key, &key_len, &count) == FAILURE)
+    {
+        return FAILURE;
+    }
+
+    redis_cmd_init_sstr(&cmdstr, 1 + (count > 0), kw, strlen(kw));
+    redis_cmd_append_sstr_key(&cmdstr, key, key_len, redis_sock, slot);
+    if (count > 0) {
+        redis_cmd_append_sstr_long(&cmdstr, (long)count);
+        *ctx = PHPREDIS_CTX_PTR;
+    }
+
+    *cmd = cmdstr.c;
+    *cmd_len = cmdstr.len;
+
+    return SUCCESS;
+}
+
 /* Attempt to pull a long expiry from a zval.  We're more restrictave than zval_get_long
  * because that function will return integers from things like open file descriptors
  * which should simply fail as a TTL */
diff --git a/redis_commands.h b/redis_commands.h
index 45f9bd9c..dc88848e 100644
--- a/redis_commands.h
+++ b/redis_commands.h
@@ -84,6 +84,9 @@ int redis_key_val_arr_cmd(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock,
 int redis_key_str_arr_cmd(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock,
     char *kw, char **cmd, int *cmd_len, short *slot, void **ctx);
 
+int redis_pop_cmd(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock,
+    char *kw, char **cmd, int *cmd_len, short *slot, void **ctx);
+
 int redis_blocking_pop_cmd(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock,
     char *kw, char **cmd, int *cmd_len, short *slot, void **ctx);
 
diff --git a/redis_legacy_arginfo.h b/redis_legacy_arginfo.h
index 858f299f..85cc8be1 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: 858f814d5b91c0829ae6b6a265a740cc037586dd */
+ * Stub hash: 9671c30926e8d581a126833360b123c8ae2dd913 */
 
 ZEND_BEGIN_ARG_INFO_EX(arginfo_class_Redis___construct, 0, 0, 0)
 	ZEND_ARG_INFO(0, options)
@@ -366,7 +366,10 @@ ZEND_BEGIN_ARG_INFO_EX(arginfo_class_Redis_lMove, 0, 0, 4)
 	ZEND_ARG_INFO(0, whereto)
 ZEND_END_ARG_INFO()
 
-#define arginfo_class_Redis_lPop arginfo_class_Redis__prefix
+ZEND_BEGIN_ARG_INFO_EX(arginfo_class_Redis_lPop, 0, 0, 1)
+	ZEND_ARG_INFO(0, key)
+	ZEND_ARG_INFO(0, count)
+ZEND_END_ARG_INFO()
 
 ZEND_BEGIN_ARG_INFO_EX(arginfo_class_Redis_lPush, 0, 0, 1)
 	ZEND_ARG_INFO(0, key)
@@ -487,7 +490,7 @@ ZEND_END_ARG_INFO()
 
 #define arginfo_class_Redis_punsubscribe arginfo_class_Redis_psubscribe
 
-#define arginfo_class_Redis_rPop arginfo_class_Redis__prefix
+#define arginfo_class_Redis_rPop arginfo_class_Redis_lPop
 
 #define arginfo_class_Redis_randomKey arginfo_class_Redis___destruct
 
@@ -551,12 +554,9 @@ ZEND_BEGIN_ARG_INFO_EX(arginfo_class_Redis_sMove, 0, 0, 3)
 	ZEND_ARG_INFO(0, value)
 ZEND_END_ARG_INFO()
 
-ZEND_BEGIN_ARG_INFO_EX(arginfo_class_Redis_sPop, 0, 0, 1)
-	ZEND_ARG_INFO(0, key)
-	ZEND_ARG_INFO(0, count)
-ZEND_END_ARG_INFO()
+#define arginfo_class_Redis_sPop arginfo_class_Redis_lPop
 
-#define arginfo_class_Redis_sRandMember arginfo_class_Redis_sPop
+#define arginfo_class_Redis_sRandMember arginfo_class_Redis_lPop
 
 #define arginfo_class_Redis_sUnion arginfo_class_Redis_del
 
diff --git a/tests/RedisTest.php b/tests/RedisTest.php
index d480e7ec..43a0cb1e 100644
--- a/tests/RedisTest.php
+++ b/tests/RedisTest.php
@@ -878,11 +878,16 @@ class Redis_Test extends TestSuite
         $this->redis->lPush('list', 'val2');
     $this->redis->rPush('list', 'val3');
 
+
     // 'list' = [ 'val2', 'val', 'val3']
 
     $this->assertEquals('val2', $this->redis->lPop('list'));
-        $this->assertEquals('val', $this->redis->lPop('list'));
-        $this->assertEquals('val3', $this->redis->lPop('list'));
+        if (version_compare($this->version, "6.2.0") < 0) {
+            $this->assertEquals('val', $this->redis->lPop('list'));
+            $this->assertEquals('val3', $this->redis->lPop('list'));
+        } else {
+            $this->assertEquals(['val', 'val3'], $this->redis->lPop('list', 2));
+        }
         $this->assertEquals(FALSE, $this->redis->lPop('list'));
 
     // testing binary data
@@ -895,7 +900,6 @@ class Redis_Test extends TestSuite
     $this->assertEquals('val3', gzuncompress($this->redis->lPop('list')));
     $this->assertEquals('val2', gzuncompress($this->redis->lPop('list')));
     $this->assertEquals('val1', gzuncompress($this->redis->lPop('list')));
-
     }
 
     // PUSH, POP : RPUSH, RPOP
@@ -913,8 +917,12 @@ class Redis_Test extends TestSuite
     // 'list' = [ 'val3', 'val', 'val2']
 
     $this->assertEquals('val2', $this->redis->rPop('list'));
-        $this->assertEquals('val', $this->redis->rPop('list'));
-        $this->assertEquals('val3', $this->redis->rPop('list'));
+        if (version_compare($this->version, "6.2.0") < 0) {
+            $this->assertEquals('val', $this->redis->rPop('list'));
+            $this->assertEquals('val3', $this->redis->rPop('list'));
+        } else {
+            $this->assertEquals(['val', 'val3'], $this->redis->rPop('list', 2));
+        }
         $this->assertEquals(FALSE, $this->redis->rPop('list'));
 
     // testing binary data