Fix review comments

docs/admin_api/user_admin_api.rst

@@ -5,39 +5,40 @@ This API returns all local user accounts.
 
 The api is::
 
-    GET /_synapse/admin/v2/users?offset=0&limit=10&guests=false
+    GET /_synapse/admin/v2/users?from=0&limit=10&guests=false
 
 including an ``access_token`` of a server admin.
-The parameters ``offset`` and ``limit`` are required only for pagination.
-Per default a ``limit`` of 100 is used. If the endpoint returns less entries
-than specified by ``limit`` then there are no more users left.
-The parameter ``name`` can be used to filter by user name.
-The parameter ``guests`` can be used to exclude guest users.
-The parameter ``deactivated`` can be used to include deactivated users.
-
+The parameters ``from`` and ``limit`` are required only for pagination.
+By default, a ``limit`` of 100 is used.
+The parameter ``user_id`` can be used to filter by user id using an SQL regexp.
+The parameter ``guests=false`` can be used to exclude guest users,
+default is to include guest users.
+The parameter ``deactivated=true`` can be used to include deactivated users,
+default is to exclude deactivated users.
+If the endpoint does not return a ``next_token`` then there are no more users left.
 It returns a JSON body like the following:
 
 .. code:: json
 
     {
         "users": [
             {
-                "name": "<user_id1>",
+                "user_id": "<user_id1>",
                 "password_hash": "<password_hash1>",
                 "is_guest": 0,
                 "admin": 0,
                 "user_type": null,
                 "deactivated": 0
             }, {
-                "name": "<user_id2>",
+                "user_id": "<user_id2>",
                 "password_hash": "<password_hash2>",
                 "is_guest": 0,
                 "admin": 1,
                 "user_type": null,
                 "deactivated": 0
             }
         ],
-        "next_token": 100
+        "next_token": "100"
     }
 
 

synapse/rest/admin/users.py

@@ -66,15 +66,14 @@ class UsersRestServletV2(RestServlet):
     """Get request to list all local users.
     This needs user to have administrator access in Synapse.
 
-    GET /_synapse/admin/v2/users?offset=0&limit=10&guests=false
+    GET /_synapse/admin/v2/users?from=0&limit=10&guests=false
 
     returns:
         200 OK with list of users if success otherwise an error.
 
-    The parameters `offset` and `limit` are required only for pagination.
-    Per default a `limit` of 100 is used. If the endpoint returns less entries
-    than specified by `limit` then there are no more users left.
-    The parameter `name` can be used to filter by user name.
+    The parameters `from` and `limit` are required only for pagination.
+    By default, a `limit` of 100 is used.
+    The parameter `user_id` can be used to filter by user id.
     The parameter `guests` can be used to exclude guest users.
     The parameter `deactivated` can be used to include deactivated users.
     """
@@ -87,16 +86,20 @@ def __init__(self, hs):
     async def on_GET(self, request):
         await assert_requester_is_admin(self.auth, request)
 
-        start = parse_integer(request, "offset", default=0)
+        start = parse_integer(request, "from", default=0)
         limit = parse_integer(request, "limit", default=100)
-        name = parse_string(request, "name", default=None)
+        user_id = parse_string(request, "user_id", default=None)
         guests = parse_boolean(request, "guests", default=True)
         deactivated = parse_boolean(request, "deactivated", default=False)
 
         users = await self.admin_handler.get_users_paginate(
-            start, limit, name, guests, deactivated
+            start, limit, user_id, guests, deactivated
         )
-        return 200, {"users": users, "next_token": start + len(users)}
+        ret = {"users": users}
+        if len(users) >= limit:
+            ret["next_token"] = str(start + len(users))
+
+        return 200, ret
 
 
 class UserRegisterServlet(RestServlet):

synapse/storage/_base.py

@@ -1534,12 +1534,12 @@ def get_cache_stream_token(self):
     def _simple_select_list_paginate(
         self,
         table,
-        filters,
-        keyvalues,
         orderby,
         start,
         limit,
         retcols,
+        filters=None,
+        keyvalues=None,
         order_direction="ASC",
         desc="_simple_select_list_paginate",
     ):
@@ -1568,12 +1568,12 @@ def _simple_select_list_paginate(
             desc,
             self._simple_select_list_paginate_txn,
             table,
-            filters,
-            keyvalues,
             orderby,
             start,
             limit,
             retcols,
+            filters=filters,
+            keyvalues=keyvalues,
             order_direction=order_direction,
         )
 
@@ -1582,32 +1582,35 @@ def _simple_select_list_paginate_txn(
         cls,
         txn,
         table,
-        filters,
-        keyvalues,
         orderby,
         start,
         limit,
         retcols,
+        filters=None,
+        keyvalues=None,
         order_direction="ASC",
     ):
         """
         Executes a SELECT query on the named table with start and limit,
         of row numbers, which may return zero or number of rows from start to limit,
         returning the result as a list of dicts.
+        Use filters to search attributes using SQL regular expressions and/or keyvalues
+        to select attributes with exact matches. All constraints are joined together
+        using 'AND'.
 
         Args:
             txn : Transaction object
             table (str): the table name
+            orderby (str): Column to order the results by.
+            start (int): Index to begin the query at.
+            limit (int): Number of results to return.
+            retcols (iterable[str]): the names of the columns to return
             filters (dict[str, T] | None):
                 column names and values to filter the rows with, or None to not
                 apply a WHERE ? LIKE ? clause.
             keyvalues (dict[str, T] | None):
                 column names and values to select the rows with, or None to not
                 apply a WHERE clause.
-            orderby (str): Column to order the results by.
-            start (int): Index to begin the query at.
-            limit (int): Number of results to return.
-            retcols (iterable[str]): the names of the columns to return
             order_direction (str): Whether the results should be ordered "ASC" or "DESC".
         Returns:
             defer.Deferred: resolves to list[dict[str, Any]]
@@ -1629,7 +1632,9 @@ def _simple_select_list_paginate_txn(
             orderby,
             order_direction,
         )
-        txn.execute(sql, list(keyvalues.values()) + [limit, start])
+        txn.execute(
+            sql, list(filters.values()) + list(keyvalues.values()) + [limit, start]
+        )
 
         return cls.cursor_to_dict(txn)
 

synapse/storage/data_stores/main/__init__.py

@@ -519,11 +519,11 @@ def get_users_paginate(
 
         return self._simple_select_list_paginate(
             table="users",
-            filters=name_filter,
-            keyvalues=attr_filter,
             orderby="name",
             start=start,
             limit=limit,
+            filters=name_filter,
+            keyvalues=attr_filter,
             retcols=[
                 "name",
                 "password_hash",

synapse/storage/data_stores/main/stats.py

@@ -260,12 +260,11 @@ def _get_statistics_for_subject_txn(
         slice_list = self._simple_select_list_paginate_txn(
             txn,
             table + "_historical",
-            None,
-            {id_col: stats_id},
             "end_ts",
             start,
             size,
             retcols=selected_columns + ["bucket_size", "end_ts"],
+            keyvalues={id_col: stats_id},
             order_direction="DESC",
         )