Sitelet https://github.com/phpredis/phpredis/commit/0c17bd27
Skip to content

Commit 0c17bd2

Browse files
michael-grundermichael-grunder
committed
Make the XREADGROUP optional COUNT and BLOCK arguments nullable.
Change the way we accept COUNT and BLOCK such that a user can pass NULL to mean "no value". This is technically a breaking change, since previously the value `-1` was used for "no value". Fixes #1560
1 parent 6e49417 commit 0c17bd2

2 files changed

Lines changed: 31 additions & 7 deletions

File tree

‎redis_commands.c‎

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3466,23 +3466,30 @@ int redis_xreadgroup_cmd(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock,
34663466
char *group, *consumer;
34673467
size_t grouplen, consumerlen;
34683468
int scount, argc;
3469-
zend_long count = -1, block = -1;
3469+
zend_long count, block;
3470+
zend_bool no_count = 1, no_block = 1;
34703471

3471-
if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "ssa|ll", &group,
3472+
if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "ssa|l!l!", &group,
34723473
&grouplen, &consumer, &consumerlen, &z_streams,
3473-
&count, &block) == FAILURE)
3474+
&count, &no_count, &block, &no_block) == FAILURE)
34743475
{
34753476
return FAILURE;
34763477
}
34773478

3479+
/* Negative COUNT or BLOCK is illegal so abort immediately */
3480+
if ((!no_count && count < 0) || (!no_block && block < 0)) {
3481+
php_error_docref(NULL TSRMLS_CC, E_WARNING, "Negative values for COUNT or BLOCK are illegal.");
3482+
return FAILURE;
3483+
}
3484+
34783485
/* Redis requires at least one stream */
34793486
kt = Z_ARRVAL_P(z_streams);
34803487
if ((scount = zend_hash_num_elements(kt)) < 1) {
34813488
return FAILURE;
34823489
}
34833490

34843491
/* Calculate argc and start constructing commands */
3485-
argc = 4 + (2 * scount) + (2 * (count > -1)) + (2 * (block > -1));
3492+
argc = 4 + (2 * scount) + (2 * !no_count) + (2 * !no_block);
34863493
REDIS_CMD_INIT_SSTR_STATIC(&cmdstr, argc, "XREADGROUP");
34873494

34883495
/* Group and consumer */
@@ -3491,13 +3498,13 @@ int redis_xreadgroup_cmd(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock,
34913498
redis_cmd_append_sstr(&cmdstr, consumer, consumerlen);
34923499

34933500
/* Append COUNT if we have it */
3494-
if (count > -1) {
3501+
if (!no_count) {
34953502
REDIS_CMD_APPEND_SSTR_STATIC(&cmdstr, "COUNT");
34963503
redis_cmd_append_sstr_long(&cmdstr, count);
34973504
}
34983505

34993506
/* Append BLOCK argument if we have it */
3500-
if (block > -1) {
3507+
if (!no_block) {
35013508
REDIS_CMD_APPEND_SSTR_STATIC(&cmdstr, "BLOCK");
35023509
redis_cmd_append_sstr_long(&cmdstr, block);
35033510
}

‎tests/RedisTest.php‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5687,12 +5687,29 @@ public function testXReadGroup() {
56875687
}
56885688
}
56895689

5690+
/* Test COUNT option with NULL (should be ignored) */
5691+
$this->addStreamsAndGroups($streams, 3, $groups, NULL);
5692+
$resp = $this->redis->xReadGroup('g1', 'consumer', $query1, NULL);
5693+
foreach ($resp as $stream => $smsg) {
5694+
$this->assertEquals(count($smsg), 3);
5695+
}
5696+
56905697
/* Finally test BLOCK with a sloppy timing test */
56915698
$t1 = $this->mstime();
56925699
$qnew = ['{s}-1' => '>', '{s}-2' => '>'];
5693-
$this->redis->xReadGroup('g1', 'c1', $qnew, -1, 100);
5700+
$this->redis->xReadGroup('g1', 'c1', $qnew, NULL, 100);
56945701
$t2 = $this->mstime();
56955702
$this->assertTrue($t2 - $t1 >= 100);
5703+
5704+
/* Make sure passing NULL to block doesn't block */
5705+
$t1 = $this->mstime();
5706+
$this->redis->xReadGroup('g1', 'c1', $qnew, NULL, NULL);
5707+
$t2 = $this->mstime();
5708+
$this->assertTrue($t2 - $t1 < 100);
5709+
5710+
/* Make sure passing bad values to BLOCK or COUNT immediately fails */
5711+
$this->assertFalse(@$this->redis->xReadGroup('g1', 'c1', $qnew, -1));
5712+
$this->assertFalse(@$this->redis->xReadGroup('g1', 'c1', $qnew, NULL, -1));
56965713
}
56975714

56985715
public function testXPending() {

0 commit comments

Comments
 (0)