From 6153477c4f71cbfadaae628e47ea022136f0131f Mon Sep 17 00:00:00 2001 From: Nicolas Favre-Felix Date: Sun, 9 Sep 2012 20:38:30 +0100 Subject: Extend getLastError to all calls Fixes GitHub issue #245. --- README.markdown | 2 +- library.c | 3 +++ tests/TestRedis.php | 12 ++++++++++-- 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/README.markdown b/README.markdown index 90b55a59..1b69bf75 100644 --- a/README.markdown +++ b/README.markdown @@ -2472,7 +2472,7 @@ $redis->script('exists', $script1, [$script2, $script3, ...]); ## getLastError ##### Description -The last error message (if any) returned from a SCRIPT call +The last error message (if any) ##### Parameters *none* ##### Return Value diff --git a/library.c b/library.c index 25b1d3aa..6a93f490 100644 --- a/library.c +++ b/library.c @@ -172,6 +172,7 @@ PHPAPI char *redis_sock_read(RedisSock *redis_sock, int *buf_len TSRMLS_DC) { char inbuf[1024]; char *resp = NULL; + size_t err_len; if(-1 == redis_check_eof(redis_sock TSRMLS_CC)) { return NULL; @@ -189,6 +190,8 @@ PHPAPI char *redis_sock_read(RedisSock *redis_sock, int *buf_len TSRMLS_DC) switch(inbuf[0]) { case '-': + err_len = strlen(inbuf+1) - 2; + redis_sock_set_err(redis_sock, inbuf+1, err_len); /* stale data */ if(memcmp(inbuf + 1, "-ERR SYNC ", 10) == 0) { zend_throw_exception(redis_exception_ce, "SYNC with master in progress", 0 TSRMLS_CC); diff --git a/tests/TestRedis.php b/tests/TestRedis.php index 663b808a..60e03b92 100644 --- a/tests/TestRedis.php +++ b/tests/TestRedis.php @@ -3023,8 +3023,16 @@ class Redis_Test extends TestSuite $this->redis->eval("not-a-lua-script"); // Now we should have an error - $this->assertTrue(strlen($this->redis->getLastError()) > 0); - } + $evalError = $this->redis->getLastError(); + $this->assertTrue(strlen($evalError) > 0); + + // test getLastError with a regular command + $this->redis->set('x', 'a'); + $this->assertFalse($this->redis->incr('x')); + $incrError = $this->redis->getLastError(); + $this->assertTrue($incrError !== $evalError); // error has changed + $this->assertTrue(strlen($incrError) > 0); + } // Helper function to compare nested results -- from the php.net array_diff page, I believe private function array_diff_recursive($aArray1, $aArray2) { -- cgit v1.2.3 From 5855cfc2ca0e2dbe5fa754e3eeb092ef4d89a7eb Mon Sep 17 00:00:00 2001 From: Nicolas Favre-Felix Date: Sun, 9 Sep 2012 22:09:27 +0100 Subject: Add clearLastError --- README.markdown | 18 ++++++++++++++++++ php_redis.h | 1 + redis.c | 27 +++++++++++++++++++++++++++ tests/TestRedis.php | 4 ++++ 4 files changed, 50 insertions(+) diff --git a/README.markdown b/README.markdown index 1b69bf75..69b7c99f 100644 --- a/README.markdown +++ b/README.markdown @@ -2484,6 +2484,24 @@ $err = $redis->getLastError(); // "ERR Error compiling script (new function): user_script:1: '=' expected near '-'" +## clearLastError +##### Description +Clear the last error message +##### Parameters +*none* +##### Return Value +*BOOL* TRUE +##### Examples +
+$redis->set('x', 'a');
+$redis->incr('x');
+$err = $redis->getLastError();
+// "ERR value is not an integer or out of range"
+$redis->clearLastError();
+$err = $redis->getLastError();
+// NULL
+
+ ## _prefix ##### Description A utility method to prefix the value with the prefix setting for phpredis. diff --git a/php_redis.h b/php_redis.h index 71e29ffd..254b0ef3 100644 --- a/php_redis.h +++ b/php_redis.h @@ -139,6 +139,7 @@ PHP_METHOD(Redis, migrate); PHP_METHOD(Redis, time); PHP_METHOD(Redis, getLastError); +PHP_METHOD(Redis, clearLastError); PHP_METHOD(Redis, _prefix); PHP_METHOD(Redis, _unserialize); diff --git a/redis.c b/redis.c index c3667fc3..0ebc3f83 100644 --- a/redis.c +++ b/redis.c @@ -219,6 +219,7 @@ static zend_function_entry redis_functions[] = { PHP_ME(Redis, migrate, NULL, ZEND_ACC_PUBLIC) PHP_ME(Redis, getLastError, NULL, ZEND_ACC_PUBLIC) + PHP_ME(Redis, clearLastError, NULL, ZEND_ACC_PUBLIC) PHP_ME(Redis, _prefix, NULL, ZEND_ACC_PUBLIC) PHP_ME(Redis, _unserialize, NULL, ZEND_ACC_PUBLIC) @@ -6161,6 +6162,32 @@ PHP_METHOD(Redis, getLastError) { } } +/* + * {{{ proto Redis::clearLastError() + */ +PHP_METHOD(Redis, clearLastError) { + zval *object; + RedisSock *redis_sock; + + // Grab our object + if(zend_parse_method_parameters(ZEND_NUM_ARGS() TSRMLS_CC, getThis(), "O", &object, redis_ce) == FAILURE) { + RETURN_FALSE; + } + // Grab socket + if(redis_sock_get(object, &redis_sock TSRMLS_CC, 0) < 0) { + RETURN_FALSE; + } + + // Clear error message + if(redis_sock->err) { + efree(redis_sock->err); + } + redis_sock->err = NULL; + + RETURN_TRUE; +} + + /* * {{{ proto Redis::time() */ diff --git a/tests/TestRedis.php b/tests/TestRedis.php index 60e03b92..192dd556 100644 --- a/tests/TestRedis.php +++ b/tests/TestRedis.php @@ -3032,6 +3032,10 @@ class Redis_Test extends TestSuite $incrError = $this->redis->getLastError(); $this->assertTrue($incrError !== $evalError); // error has changed $this->assertTrue(strlen($incrError) > 0); + + // clear error + $this->redis->clearLastError(); + $this->assertTrue($this->redis->getLastError() === NULL); } // Helper function to compare nested results -- from the php.net array_diff page, I believe -- cgit v1.2.3 From 6dc3ad6cb3bbadb5daabb53edbda093cf21a03d2 Mon Sep 17 00:00:00 2001 From: Nicolas Favre-Felix Date: Mon, 10 Sep 2012 00:29:44 +0100 Subject: Fix RedisArray::_rehash to support closures * Add "f" parameter in _rehash() * Call object with new method * Add rehash test with closure --- arrays.markdown | 2 +- redis_array.c | 14 ++++++++++---- redis_array_impl.c | 41 +++++++++++++++++++++++++++-------------- redis_array_impl.h | 2 +- tests/array-tests.php | 8 ++++++++ 5 files changed, 47 insertions(+), 20 deletions(-) diff --git a/arrays.markdown b/arrays.markdown index b7031fed..4c1f899d 100644 --- a/arrays.markdown +++ b/arrays.markdown @@ -73,7 +73,7 @@ For instance, the keys “{user:1}:name” and “{user:1}:email” will be stor ## Migrating keys -When a node is added or removed from a ring, RedisArray instances must be instanciated with a “previous” list of nodes. A single call to `$ra->_rehash()` causes all the keys to be redistributed according to the new list of nodes. Passing a callback function to `_rehash()` makes it possible to track the progress of that operation: the function is called with a node name and a number of keys that will be examined. +When a node is added or removed from a ring, RedisArray instances must be instanciated with a “previous” list of nodes. A single call to `$ra->_rehash()` causes all the keys to be redistributed according to the new list of nodes. Passing a callback function to `_rehash()` makes it possible to track the progress of that operation: the function is called with a node name and a number of keys that will be examined, e.g. `_rehash(function ($host, $count){ ... });`. It is possible to automate this process, by setting `'autorehash' => TRUE` in the constructor options. This will cause keys to be migrated when they need to be read from the previous array. diff --git a/redis_array.c b/redis_array.c index 7964c818..380c53c1 100644 --- a/redis_array.c +++ b/redis_array.c @@ -499,11 +499,13 @@ PHP_METHOD(RedisArray, _distributor) PHP_METHOD(RedisArray, _rehash) { - zval *object, *z_cb = NULL; + zval *object; RedisArray *ra; + zend_fcall_info z_cb; + zend_fcall_info_cache z_cb_cache; - if (zend_parse_method_parameters(ZEND_NUM_ARGS() TSRMLS_CC, getThis(), "O|z", - &object, redis_array_ce, &z_cb) == FAILURE) { + if (zend_parse_method_parameters(ZEND_NUM_ARGS() TSRMLS_CC, getThis(), "O|f", + &object, redis_array_ce, &z_cb, &z_cb_cache) == FAILURE) { RETURN_FALSE; } @@ -511,7 +513,11 @@ PHP_METHOD(RedisArray, _rehash) RETURN_FALSE; } - ra_rehash(ra, z_cb TSRMLS_CC); + if (ZEND_NUM_ARGS() == 0) { + ra_rehash(ra, NULL, NULL TSRMLS_CC); + } else { + ra_rehash(ra, &z_cb, &z_cb_cache TSRMLS_CC); + } } static void multihost_distribute(INTERNAL_FUNCTION_PARAMETERS, const char *method_name) diff --git a/redis_array_impl.c b/redis_array_impl.c index 7968a8b5..d5370c82 100644 --- a/redis_array_impl.c +++ b/redis_array_impl.c @@ -1105,24 +1105,37 @@ ra_move_key(const char *key, int key_len, zval *z_from, zval *z_to TSRMLS_DC) { } /* callback with the current progress, with hostname and count */ -static void zval_rehash_callback(zval *z_cb, const char *hostname, long count TSRMLS_DC) { +static void zval_rehash_callback(zend_fcall_info *z_cb, zend_fcall_info_cache *z_cb_cache, + const char *hostname, long count TSRMLS_DC) { - zval z_ret, *z_args[2]; + zval *z_ret = NULL, **z_args[2]; + zval *z_host, *z_count; + + z_cb->retval_ptr_ptr = &z_ret; + z_cb->params = &z_args; + z_cb->param_count = 2; + z_cb->no_separation = 0; /* run cb(hostname, count) */ - MAKE_STD_ZVAL(z_args[0]); - ZVAL_STRING(z_args[0], hostname, 0); - MAKE_STD_ZVAL(z_args[1]); - ZVAL_LONG(z_args[1], count); - call_user_function(EG(function_table), NULL, z_cb, &z_ret, 2, z_args TSRMLS_CC); + MAKE_STD_ZVAL(z_host); + ZVAL_STRING(z_host, hostname, 0); + z_args[0] = &z_host; + MAKE_STD_ZVAL(z_count); + ZVAL_LONG(z_count, count); + z_args[1] = &z_count; + + zend_call_function(z_cb, z_cb_cache TSRMLS_CC); /* cleanup */ - efree(z_args[0]); - efree(z_args[1]); + efree(z_host); + efree(z_count); + if(z_ret) + efree(z_ret); } static void -ra_rehash_server(RedisArray *ra, zval *z_redis, const char *hostname, zend_bool b_index, zval *z_cb TSRMLS_DC) { +ra_rehash_server(RedisArray *ra, zval *z_redis, const char *hostname, zend_bool b_index, + zend_fcall_info *z_cb, zend_fcall_info_cache *z_cb_cache TSRMLS_DC) { char **keys; int *key_lens; @@ -1138,8 +1151,8 @@ ra_rehash_server(RedisArray *ra, zval *z_redis, const char *hostname, zend_bool } /* callback */ - if(z_cb) { - zval_rehash_callback(z_cb, hostname, count TSRMLS_CC); + if(z_cb && z_cb_cache) { + zval_rehash_callback(z_cb, z_cb_cache, hostname, count TSRMLS_CC); } /* for each key, redistribute */ @@ -1163,7 +1176,7 @@ ra_rehash_server(RedisArray *ra, zval *z_redis, const char *hostname, zend_bool } void -ra_rehash(RedisArray *ra, zval *z_cb TSRMLS_DC) { +ra_rehash(RedisArray *ra, zend_fcall_info *z_cb, zend_fcall_info_cache *z_cb_cache TSRMLS_DC) { int i; @@ -1172,7 +1185,7 @@ ra_rehash(RedisArray *ra, zval *z_cb TSRMLS_DC) { return; /* TODO: compare the two rings for equality */ for(i = 0; i < ra->prev->count; ++i) { - ra_rehash_server(ra, ra->prev->redis[i], ra->prev->hosts[i], ra->index, z_cb TSRMLS_CC); + ra_rehash_server(ra, ra->prev->redis[i], ra->prev->hosts[i], ra->index, z_cb, z_cb_cache TSRMLS_CC); } } diff --git a/redis_array_impl.h b/redis_array_impl.h index 3cbe3f02..0e00258c 100644 --- a/redis_array_impl.h +++ b/redis_array_impl.h @@ -24,6 +24,6 @@ void ra_index_discard(zval *z_redis, zval *return_value TSRMLS_DC); void ra_index_unwatch(zval *z_redis, zval *return_value TSRMLS_DC); zend_bool ra_is_write_cmd(RedisArray *ra, const char *cmd, int cmd_len); -void ra_rehash(RedisArray *ra, zval *z_cb TSRMLS_DC); +void ra_rehash(RedisArray *ra, zend_fcall_info *z_cb, zend_fcall_info_cache *z_cb_cache TSRMLS_DC); #endif diff --git a/tests/array-tests.php b/tests/array-tests.php index 2429cab9..2f72155a 100644 --- a/tests/array-tests.php +++ b/tests/array-tests.php @@ -289,6 +289,14 @@ class Redis_Rehashing_Test extends TestSuite $this->ra->_rehash(); // this will redistribute the keys } + public function testRehashWithCallback() { + $total = 0; + $this->ra->_rehash(function ($host, $count) use (&$total) { + $total += $count; + }); + $this->assertTrue($total > 0); + } + public function testReadRedistributedKeys() { $this->readAllvalues(); // we shouldn't have any missed reads now. } -- cgit v1.2.3