Skip to content

Commit

Permalink
Fixes for Redis extension and associated test suite
Browse files Browse the repository at this point in the history
Summary: * Fix a couple of tests that were broken by 91b1918 adding two lines to the
  wrong expect file (list.php.expect instead of info.php.expect).
* Fixed hphp/test/slow/ext_redis/watch.php.expect to expect the correct
  behavior. (Tested by verifying with Zend + phpredis.)
* Make the key prefix set by the test suite include the md5 of the file path
  plus UNIX timestamp representing the test suite initialization. This makes
  the tests not break when the path exceeds the redis key length limit and it
  makes repeated test runs not conflict with one another.
* Make Redis::sRandMember apply the key prefix (if one exists).
* Make Redis::get() and Redis::h{m}get() return false rather than null for
  missing keys or fields, matching the behavior of phpredis.

Minimal test case for the false / null behavior:

    <?php
    $redis = new Redis;
    $redis->connect("localhost");
    var_dump($redis->get("nonexistent-key"));
    $redis->hMset('ice_cream', array('flavor' => 'vanilla'));
    var_dump($redi

Closes #3318

Reviewed By: bsimmers, ptarjan

Differential Revision: D1467674

Pulled By: svcscm
  • Loading branch information
atdt authored and facebook-github-bot committed Jul 31, 2014
1 parent 31de467 commit 47b3503
Show file tree
Hide file tree
Showing 10 changed files with 22 additions and 19 deletions.
11 changes: 7 additions & 4 deletions hphp/system/php/redis/Redis.php
Expand Up @@ -369,7 +369,7 @@ public function hMSet($key, array $pairs) {
/* Sets ---------------------------------------------------------------- */

public function sRandMember($key, $count = null) {
$args = [$key];
$args = [$this->prefix($key)];
if ($count !== null) {
$args[] = $count;
}
Expand Down Expand Up @@ -1220,8 +1220,11 @@ private function doProcessVariantResponse() {
protected function processSerializedResponse() {
if ($this->mode === self::ATOMIC) {
$resp = $this->sockReadData($type);
if ($resp === null) {
return false;
}
return (($type === self::TYPE_LINE) OR ($type === self::TYPE_BULK))
? $this->unserialize($resp) : null;
? $this->unserialize($resp) : false;
}
$this->multiHandler[] = [ 'cb' => [$this,'processSerializedResponse'] ];
if (($this->mode === self::MULTI) && !$this->processQueuedResponse()) {
Expand Down Expand Up @@ -1307,7 +1310,7 @@ protected function processVectorResponse($unser = 0) {
if ($unser AND (($lineNo % $unser) == 0)) {
$val = $this->unserialize($val);
}
$ret[] = $val;
$ret[] = $val !== null ? $val : false;
}
return $ret;
}
Expand Down Expand Up @@ -1370,7 +1373,7 @@ protected function processAssocResponse(array $keys, $unser_val = true) {
if ($unser_val) {
$val = $this->unserialize($val);
}
$ret[$key] = $val;
$ret[$key] = $val !== null ? $val : false;
}
return $ret;
}
Expand Down
4 changes: 2 additions & 2 deletions hphp/test/slow/ext_redis/hmsetget.php.expect
Expand Up @@ -27,7 +27,7 @@ array(3) {
string(3) "bar"
string(5) "boom1"
string(5) "boom2"
NULL
bool(false)
array(4) {
["foo"]=>
string(3) "bar"
Expand All @@ -36,5 +36,5 @@ array(4) {
["baz2"]=>
string(5) "boom2"
["nofield"]=>
NULL
bool(false)
}
2 changes: 1 addition & 1 deletion hphp/test/slow/ext_redis/hsetget.php.expect
Expand Up @@ -20,4 +20,4 @@ array(2) {
}
string(3) "bar"
string(4) "boom"
NULL
bool(false)
2 changes: 2 additions & 0 deletions hphp/test/slow/ext_redis/info.php.expect
Expand Up @@ -4,3 +4,5 @@ bool(true)
bool(true)
bool(true)
string(17) "hhvm-redis-client"
bool(true)
bool(true)
2 changes: 0 additions & 2 deletions hphp/test/slow/ext_redis/list.php.expect
Expand Up @@ -13,5 +13,3 @@ int(2)
string(11) "easy as 123"
string(5) "apple"
string(11) "easy as 123"
bool(true)
bool(true)
6 changes: 3 additions & 3 deletions hphp/test/slow/ext_redis/redis.inc
Expand Up @@ -21,7 +21,7 @@ function NewRedisTestInstance($status = false) {
}

function GetTestKeyName($test, $rand = false) {
$name = 'REDIS_TEST:' . preg_replace('/[^a-zA-Z0-9]/', '-', $test);
if ($rand) $name .= ':' . rand(0,1000000);
return $name;
static $ts;
$ts = $ts ?: time();
return 'REDIS_TEST:' . md5( $test . $ts . ( $rand ? '' : rand(0,1000000) ) );
}
6 changes: 3 additions & 3 deletions hphp/test/slow/ext_redis/setExtended.php.expect
Expand Up @@ -5,11 +5,11 @@ bool(false)
bool(true)
bool(false)
bool(true)
NULL
bool(false)
bool(true)
bool(true)
NULL
bool(false)
bool(true)
string(2) "10"
NULL
bool(false)
Redis Exception: Invalid set options: nx and xx may not be specified at the same time
2 changes: 1 addition & 1 deletion hphp/test/slow/ext_redis/srandmember.php.expect
Expand Up @@ -2,6 +2,6 @@ string(3) "bar"
array(0) {
}
array(1) {
[0] =>
[0]=>
string(3) "bar"
}
4 changes: 2 additions & 2 deletions hphp/test/slow/ext_redis/watch.php
Expand Up @@ -16,7 +16,7 @@
var_dump($r1->watch($key1)); //true
$checkValue = $r1->get($key1);
var_dump($checkValue); //6379
if ($checkValue !== null && $checkValue == $value) {
if ($checkValue !== false && $checkValue == $value) {
var_dump($r1->multi()->del($key1)->exec()); //[1]
}

Expand All @@ -25,6 +25,6 @@
$checkValue = $r1->get($key1);
var_dump($r2->set($key1, 'different value'));
var_dump($checkValue);
if ($checkValue !== null && $checkValue == $value) {
if ($checkValue !== false && $checkValue == $value) {
var_dump($r1->multi()->del($key1)->exec());
}
2 changes: 1 addition & 1 deletion hphp/test/slow/ext_redis/watch.php.expect
Expand Up @@ -11,5 +11,5 @@ bool(true)
string(4) "6379"
array(1) {
[0]=>
int(0)
int(1)
}

0 comments on commit 47b3503

Please sign in to comment.