CLIENT LIST fix to allow multiple client_ids (#1563)

* CLIENT LIST fix to allow multiple client_ids

Support for CLIENT KILL with the USER filter

Part of #1546

* test fix

redis/commands.py

@@ -302,15 +302,16 @@ def client_kill(self, address):
         return self.execute_command('CLIENT KILL', address)
 
     def client_kill_filter(self, _id=None, _type=None, addr=None,
-                           skipme=None, laddr=None):
+                           skipme=None, laddr=None, user=None):
         """
         Disconnects client(s) using a variety of filter options
         :param id: Kills a client by its unique ID field
         :param type: Kills a client by type where type is one of 'normal',
         'master', 'slave' or 'pubsub'
         :param addr: Kills a client by its 'address:port'
         :param skipme: If True, then the client calling the command
-        :param laddr: Kills a cient by its 'local (bind)  address:port'
+        :param laddr: Kills a client by its 'local (bind)  address:port'
+        :param user: Kills a client for a specific user name
         will not get killed even if it is identified by one of the filter
         options. If skipme is not provided, the server defaults to skipme=True
         """
@@ -334,6 +335,8 @@ def client_kill_filter(self, _id=None, _type=None, addr=None,
             args.extend((b'ADDR', addr))
         if laddr is not None:
             args.extend((b'LADDR', laddr))
+        if user is not None:
+            args.extend((b'USER', user))
         if not args:
             raise DataError("CLIENT KILL <filter> <value> ... ... <filter> "
                             "<value> must specify at least one filter")
@@ -346,7 +349,7 @@ def client_info(self):
         """
         return self.execute_command('CLIENT INFO')
 
-    def client_list(self, _type=None, client_id=None):
+    def client_list(self, _type=None, client_id=[]):
         """
         Returns a list of currently connected clients.
         If type of client specified, only that type will be returned.
@@ -362,9 +365,11 @@ def client_list(self, _type=None, client_id=None):
                                 client_types,))
             args.append(b'TYPE')
             args.append(_type)
-        if client_id is not None:
+        if not isinstance(client_id, list):
+            raise DataError("client_id must be a list")
+        if client_id != []:
             args.append(b"ID")
-            args.append(client_id)
+            args.append(' '.join(client_id))
         return self.execute_command('CLIENT LIST', *args)
 
     def client_getname(self):

tests/test_commands.py

@@ -321,13 +321,19 @@ def test_client_list_type(self, r):
             assert isinstance(clients, list)
 
     @skip_if_server_version_lt('6.2.0')
-    def test_client_list_client_id(self, r):
+    def test_client_list_client_id(self, r, request):
         clients = r.client_list()
-        client_id = clients[0]['id']
-        clients = r.client_list(client_id=client_id)
+        clients = r.client_list(client_id=[clients[0]['id']])
         assert len(clients) == 1
         assert 'addr' in clients[0]
 
+        # testing multiple client ids
+        _get_client(redis.Redis, request, flushdb=False)
+        _get_client(redis.Redis, request, flushdb=False)
+        _get_client(redis.Redis, request, flushdb=False)
+        clients_listed = r.client_list(client_id=clients[:-1])
+        assert len(clients_listed) > 1
+
     @skip_if_server_version_lt('5.0.0')
     def test_client_id(self, r):
         assert r.client_id() > 0
@@ -448,6 +454,19 @@ def test_client_kill_filter_by_laddr(self, r, r2):
         client_2_addr = clients_by_name['redis-py-c2'].get('laddr')
         assert r.client_kill_filter(laddr=client_2_addr)
 
+    @skip_if_server_version_lt('2.8.12')
+    def test_client_kill_filter_by_user(self, r, request):
+        killuser = 'user_to_kill'
+        r.acl_setuser(killuser, enabled=True, reset=True,
+                      commands=['+get', '+set', '+select'],
+                      keys=['cache:*'], nopass=True)
+        _get_client(redis.Redis, request, flushdb=False, username=killuser)
+        r.client_kill_filter(user=killuser)
+        clients = r.client_list()
+        for c in clients:
+            assert c['user'] != killuser
+        r.acl_deluser(killuser)
+
     @skip_if_server_version_lt('2.9.50')
     def test_client_pause(self, r):
         assert r.client_pause(1)