Properly handle CLIENT variants :-)

Author: michael-grunder

redis_cluster.c

@@ -2703,6 +2703,24 @@ PHP_METHOD(RedisCluster, client) {
     slot = cluster_cmd_get_slot(c, z_node TSRMLS_CC);
     if(slot<0) RETURN_FALSE;
 
+    /* Our return type and reply callback is different for all subcommands */
+    if (opt_len == 4 && !strncasecmp(opt, "list", 4)) {
+        rtype = CLUSTER_IS_ATOMIC(c) ? TYPE_BULK : TYPE_LINE;
+        cb = cluster_client_list_resp;
+    } else if ((opt_len == 4 && !strncasecmp(opt, "kill", 4)) ||
+               (opt_len == 7 && !strncasecmp(opt, "setname", 7))) 
+    {
+        rtype = TYPE_LINE;
+        cb = cluster_bool_resp;
+    } else if (opt_len == 7 && !strncasecmp(opt, "getname", 7)) {
+        rtype = CLUSTER_IS_ATOMIC(c) ? TYPE_BULK : TYPE_LINE;
+        cb = cluster_bulk_resp;
+    } else {
+        php_error_docref(NULL TSRMLS_CC, E_WARNING,
+            "Invalid CLIENT subcommand (LIST, KILL, GETNAME, and SETNAME are valid");
+        RETURN_FALSE;
+    }
+
     /* Construct the command */
     if (ZEND_NUM_ARGS() == 3) {
         cmd_len = redis_cmd_format_static(&cmd, "CLIENT", "ss", opt, opt_len,
@@ -2714,27 +2732,20 @@ PHP_METHOD(RedisCluster, client) {
         RETURN_FALSE;
     }
 
-    rtype = CLUSTER_IS_ATOMIC(c) ? TYPE_BULK : TYPE_LINE;
+    /* Attempt to write our command */
     if (cluster_send_slot(c, slot, cmd, cmd_len, rtype TSRMLS_CC)<0) {
         zend_throw_exception(redis_cluster_exception_ce,
             "Unable to send CLIENT command to specific node", 0 TSRMLS_CC);
         efree(cmd);
         RETURN_FALSE;
     }
 
-    /* Handle client list and anything else differently */
-    if (opt_len == 4 && !strncasecmp(opt, "list", 4)) {
-        cb = cluster_client_list_resp;
-    } else {
-        cb = cluster_variant_resp;
-    }
-
     /* Now enqueue or process response */
     if (CLUSTER_IS_ATOMIC(c)) {
-        cluster_client_list_resp(INTERNAL_FUNCTION_PARAM_PASSTHRU, c, NULL);
+        cb(INTERNAL_FUNCTION_PARAM_PASSTHRU, c, NULL);
     } else {
         void *ctx = NULL;
-        CLUSTER_ENQUEUE_RESPONSE(c, slot, cluster_client_list_resp, ctx);
+        CLUSTER_ENQUEUE_RESPONSE(c, slot, cb, ctx);
     }
 
     efree(cmd);

tests/RedisClusterTest.php

@@ -33,9 +33,6 @@ public function testWait()     { return $this->markTestSkipped(); }
     public function testSelect()   { return $this->markTestSkipped(); }
     public function testReconnectSelect() { return $this->markTestSkipped(); }
 
-    /* Skips for now, which need attention */
-    public function testClient()   { return $this->markTestSkipped(); }
-
     /* Load our seeds on construction */
     public function __construct() {
         $str_nodemap_file = dirname($_SERVER['PHP_SELF']) . '/nodes/nodemap';
@@ -125,6 +122,30 @@ public function testInfo() {
         }
     }
 
+    public function testClient() {
+        $str_key = 'key-' . rand(1,100);
+
+        $this->assertTrue($this->redis->client($str_key, 'setname', 'cluster_tests'));
+
+        $arr_clients = $this->redis->client($str_key, 'list');
+        $this->assertTrue(is_array($arr_clients));
+
+        /* Find us in the list */
+        $str_addr = NULL;
+        foreach ($arr_clients as $arr_client) {
+            if ($arr_client['name'] == 'cluster_tests') {
+                $str_addr = $arr_client['addr'];
+                break;
+            }
+        }
+
+        /* We should be in there */
+        $this->assertFalse(empty($str_addr));
+
+        /* Kill our own client! */
+        $this->assertTrue($this->redis->client($str_key, 'kill', $str_addr));
+    }
+
     public function testTime() {
         $time_arr = $this->redis->time("k:" . rand(1,100));
         $this->assertTrue(is_array($time_arr) && count($time_arr) == 2 &&