From 5cb30fb2a65abc9d31ff58af6da0fbb6be4ec153 Mon Sep 17 00:00:00 2001 From: michael-grunder Date: Sun, 12 May 2019 18:49:33 -0700 Subject: [PATCH] Adds OPT_REPLY_LITERAL for rawCommand and EVAL Adds an option to process the actual strings in simple string replies as opposed to translating them to `true`. This only applies to `rawCommand` and `eval` because as far as I know know vanilla Redis command attaches any information besides `OK` to simple string replies. Addresses #1550 --- cluster_library.c | 6 ++++++ cluster_library.h | 4 +++- common.h | 2 ++ library.c | 9 +++++++++ library.h | 1 + redis.c | 7 ++++--- redis_cluster.c | 8 ++++---- redis_commands.c | 6 ++++++ tests/RedisClusterTest.php | 20 ++++++++++++++++++++ tests/RedisTest.php | 20 ++++++++++++++++++++ 10 files changed, 75 insertions(+), 8 deletions(-) diff --git a/cluster_library.c b/cluster_library.c index 7dd104f..dd1e0bb 100644 --- a/cluster_library.c +++ b/cluster_library.c @@ -2098,6 +2098,12 @@ PHP_REDIS_API void cluster_variant_resp(INTERNAL_FUNCTION_PARAMETERS, redisClust cluster_variant_resp_generic(INTERNAL_FUNCTION_PARAM_PASSTHRU, c, 0, ctx); } +PHP_REDIS_API void cluster_variant_raw_resp(INTERNAL_FUNCTION_PARAMETERS, redisCluster *c, + void *ctx) +{ + cluster_variant_resp_generic(INTERNAL_FUNCTION_PARAM_PASSTHRU, c, c->flags->reply_literal, ctx); +} + PHP_REDIS_API void cluster_variant_resp_strings(INTERNAL_FUNCTION_PARAMETERS, redisCluster *c, void *ctx) { diff --git a/cluster_library.h b/cluster_library.h index 4522330..3b3b4a7 100644 --- a/cluster_library.h +++ b/cluster_library.h @@ -431,10 +431,12 @@ PHP_REDIS_API void cluster_sub_resp(INTERNAL_FUNCTION_PARAMETERS, redisCluster * PHP_REDIS_API void cluster_unsub_resp(INTERNAL_FUNCTION_PARAMETERS, redisCluster *c, void *ctx); -/* Generic/Variant handler for stuff like EVAL */ PHP_REDIS_API void cluster_variant_resp(INTERNAL_FUNCTION_PARAMETERS, redisCluster *c, void *ctx); +PHP_REDIS_API void cluster_variant_raw_resp(INTERNAL_FUNCTION_PARAMETERS, + redisCluster *c, void *ctx); + PHP_REDIS_API void cluster_variant_resp_strings(INTERNAL_FUNCTION_PARAMETERS, redisCluster *c, void *ctx); diff --git a/common.h b/common.h index 2208cc4..6b75a81 100644 --- a/common.h +++ b/common.h @@ -78,6 +78,7 @@ typedef enum _PUBSUB_TYPE { #define REDIS_OPT_FAILOVER 5 #define REDIS_OPT_TCP_KEEPALIVE 6 #define REDIS_OPT_COMPRESSION 7 +#define REDIS_OPT_REPLY_LITERAL 8 /* cluster options */ #define REDIS_FAILOVER_NONE 0 @@ -272,6 +273,7 @@ typedef struct { int scan; int readonly; + int reply_literal; int tcp_keepalive; } RedisSock; /* }}} */ diff --git a/library.c b/library.c index 0f3d9f5..9c38313 100644 --- a/library.c +++ b/library.c @@ -1684,6 +1684,7 @@ redis_sock_create(char *host, int host_len, unsigned short port, redis_sock->readonly = 0; redis_sock->tcp_keepalive = 0; + redis_sock->reply_literal = 0; return redis_sock; } @@ -2558,6 +2559,14 @@ variant_reply_generic(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock, return 0; } +PHP_REDIS_API int +redis_read_raw_variant_reply(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock, + zval *z_tab, void *ctx) +{ + return variant_reply_generic(INTERNAL_FUNCTION_PARAM_PASSTHRU, redis_sock, + redis_sock->reply_literal, z_tab, ctx); +} + PHP_REDIS_API int redis_read_variant_reply(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock, zval *z_tab, void *ctx) diff --git a/library.h b/library.h index 3028711..1620429 100644 --- a/library.h +++ b/library.h @@ -116,6 +116,7 @@ PHP_REDIS_API int redis_read_reply_type(RedisSock *redis_sock, REDIS_REPLY_TYPE PHP_REDIS_API int redis_read_variant_bulk(RedisSock *redis_sock, int size, zval *z_ret TSRMLS_DC); PHP_REDIS_API int redis_read_multibulk_recursive(RedisSock *redis_sock, int elements, int status_strings, zval *z_ret TSRMLS_DC); PHP_REDIS_API int redis_read_variant_reply(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock, zval *z_tab, void *ctx); +PHP_REDIS_API int redis_read_raw_variant_reply(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock, zval *z_tab, void *ctx); PHP_REDIS_API int redis_read_variant_reply_strings(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock, zval *z_tab, void *ctx); PHP_REDIS_API void redis_client_list_reply(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock, zval *z_tab); diff --git a/redis.c b/redis.c index ef54b4a..dc315e3 100644 --- a/redis.c +++ b/redis.c @@ -666,6 +666,7 @@ static void add_class_constants(zend_class_entry *ce, int is_cluster TSRMLS_DC) zend_declare_class_constant_long(ce, ZEND_STRL("OPT_READ_TIMEOUT"), REDIS_OPT_READ_TIMEOUT TSRMLS_CC); zend_declare_class_constant_long(ce, ZEND_STRL("OPT_TCP_KEEPALIVE"), REDIS_OPT_TCP_KEEPALIVE TSRMLS_CC); zend_declare_class_constant_long(ce, ZEND_STRL("OPT_COMPRESSION"), REDIS_OPT_COMPRESSION TSRMLS_CC); + zend_declare_class_constant_long(ce, ZEND_STRL("OPT_REPLY_LITERAL"), REDIS_OPT_REPLY_LITERAL); /* serializer */ zend_declare_class_constant_long(ce, ZEND_STRL("SERIALIZER_NONE"), REDIS_SERIALIZER_NONE TSRMLS_CC); @@ -3007,12 +3008,12 @@ PHP_METHOD(Redis, pubsub) { /* {{{ proto variant Redis::eval(string script, [array keys, long num_keys]) */ PHP_METHOD(Redis, eval) { - REDIS_PROCESS_KW_CMD("EVAL", redis_eval_cmd, redis_read_variant_reply); + REDIS_PROCESS_KW_CMD("EVAL", redis_eval_cmd, redis_read_raw_variant_reply); } /* {{{ proto variant Redis::evalsha(string sha1, [array keys, long num_keys]) */ PHP_METHOD(Redis, evalsha) { - REDIS_PROCESS_KW_CMD("EVALSHA", redis_eval_cmd, redis_read_variant_reply); + REDIS_PROCESS_KW_CMD("EVALSHA", redis_eval_cmd, redis_read_raw_variant_reply); } /* {{{ proto status Redis::script('flush') @@ -3384,7 +3385,7 @@ PHP_METHOD(Redis, rawcommand) { /* Execute our command */ REDIS_PROCESS_REQUEST(redis_sock, cmd, cmd_len); if (IS_ATOMIC(redis_sock)) { - redis_read_variant_reply(INTERNAL_FUNCTION_PARAM_PASSTHRU,redis_sock,NULL,NULL); + redis_read_raw_variant_reply(INTERNAL_FUNCTION_PARAM_PASSTHRU,redis_sock,NULL,NULL); } REDIS_PROCESS_RESPONSE(redis_read_variant_reply); } diff --git a/redis_cluster.c b/redis_cluster.c index c234a20..346ee88 100644 --- a/redis_cluster.c +++ b/redis_cluster.c @@ -1989,13 +1989,13 @@ PHP_METHOD(RedisCluster, punsubscribe) { /* {{{ proto mixed RedisCluster::eval(string script, [array args, int numkeys) */ PHP_METHOD(RedisCluster, eval) { - CLUSTER_PROCESS_KW_CMD("EVAL", redis_eval_cmd, cluster_variant_resp, 0); + CLUSTER_PROCESS_KW_CMD("EVAL", redis_eval_cmd, cluster_variant_raw_resp, 0); } /* }}} */ /* {{{ proto mixed RedisCluster::evalsha(string sha, [array args, int numkeys]) */ PHP_METHOD(RedisCluster, evalsha) { - CLUSTER_PROCESS_KW_CMD("EVALSHA", redis_eval_cmd, cluster_variant_resp, 0); + CLUSTER_PROCESS_KW_CMD("EVALSHA", redis_eval_cmd, cluster_variant_raw_resp, 0); } /* }}} */ @@ -3161,10 +3161,10 @@ PHP_METHOD(RedisCluster, rawcommand) { /* Process variant response */ if (CLUSTER_IS_ATOMIC(c)) { - cluster_variant_resp(INTERNAL_FUNCTION_PARAM_PASSTHRU, c, NULL); + cluster_variant_raw_resp(INTERNAL_FUNCTION_PARAM_PASSTHRU, c, NULL); } else { void *ctx = NULL; - CLUSTER_ENQUEUE_RESPONSE(c, slot, cluster_variant_resp, ctx); + CLUSTER_ENQUEUE_RESPONSE(c, slot, cluster_variant_raw_resp, ctx); } efree(cmd); diff --git a/redis_commands.c b/redis_commands.c index 829bfb0..77c7366 100644 --- a/redis_commands.c +++ b/redis_commands.c @@ -3856,6 +3856,8 @@ void redis_getoption_handler(INTERNAL_FUNCTION_PARAMETERS, RETURN_LONG(redis_sock->tcp_keepalive); case REDIS_OPT_SCAN: RETURN_LONG(redis_sock->scan); + case REDIS_OPT_REPLY_LITERAL: + RETURN_LONG(redis_sock->reply_literal); case REDIS_OPT_FAILOVER: RETURN_LONG(c->failover); default: @@ -3896,6 +3898,10 @@ void redis_setoption_handler(INTERNAL_FUNCTION_PARAMETERS, RETURN_TRUE; } break; + case REDIS_OPT_REPLY_LITERAL: + val_long = zval_get_long(val); + redis_sock->reply_literal = val_long != 0; + RETURN_TRUE; case REDIS_OPT_COMPRESSION: val_long = zval_get_long(val); if (val_long == REDIS_COMPRESSION_NONE diff --git a/tests/RedisClusterTest.php b/tests/RedisClusterTest.php index 02e7820..ead4ab9 100644 --- a/tests/RedisClusterTest.php +++ b/tests/RedisClusterTest.php @@ -609,6 +609,26 @@ class Redis_Cluster_Test extends Redis_Test { return call_user_func_array([$this->redis, 'rawCommand'], $args); } + /* Test that rawCommand and EVAL can be configured to return simple string values */ + public function testReplyLiteral() { + $this->redis->setOption(Redis::OPT_REPLY_LITERAL, false); + $this->assertTrue($this->redis->rawCommand('foo', 'set', 'foo', 'bar')); + $this->assertTrue($this->redis->eval("return redis.call('set', KEYS[1], 'bar')", ['foo'], 1)); + + $rv = $this->redis->eval("return {redis.call('set', KEYS[1], 'bar'), redis.call('ping')}", ['foo'], 1); + $this->assertEquals([true, true], $rv); + + $this->redis->setOption(Redis::OPT_REPLY_LITERAL, true); + $this->assertEquals('OK', $this->redis->rawCommand('foo', 'set', 'foo', 'bar')); + $this->assertEquals('OK', $this->redis->eval("return redis.call('set', KEYS[1], 'bar')", ['foo'], 1)); + + $rv = $this->redis->eval("return {redis.call('set', KEYS[1], 'bar'), redis.call('ping')}", ['foo'], 1); + $this->assertEquals(['OK', 'PONG'], $rv); + + // Reset + $this->redis->setOption(Redis::OPT_REPLY_LITERAL, false); + } + public function testSession() { @ini_set('session.save_handler', 'rediscluster'); diff --git a/tests/RedisTest.php b/tests/RedisTest.php index 903c381..903a97b 100644 --- a/tests/RedisTest.php +++ b/tests/RedisTest.php @@ -4825,6 +4825,26 @@ class Redis_Test extends TestSuite } + public function testReplyLiteral() { + $this->redis->setOption(Redis::OPT_REPLY_LITERAL, false); + $this->assertTrue($this->redis->rawCommand('set', 'foo', 'bar')); + $this->assertTrue($this->redis->eval("return redis.call('set', 'foo', 'bar')", [], 0)); + + $rv = $this->redis->eval("return {redis.call('set', KEYS[1], 'bar'), redis.call('ping')}", ['foo'], 1); + $this->assertEquals([true, true], $rv); + + $this->redis->setOption(Redis::OPT_REPLY_LITERAL, true); + $this->assertEquals('OK', $this->redis->rawCommand('set', 'foo', 'bar')); + $this->assertEquals('OK', $this->redis->eval("return redis.call('set', 'foo', 'bar')", [], 0)); + + // Nested + $rv = $this->redis->eval("return {redis.call('set', KEYS[1], 'bar'), redis.call('ping')}", ['foo'], 1); + $this->assertEquals(['OK', 'PONG'], $rv); + + // Reset + $this->redis->setOption(Redis::OPT_REPLY_LITERAL, false); + } + public function testReconnectSelect() { $key = 'reconnect-select'; $value = 'Has been set!';