From 2a6111fd80b23ff1e7c768d3d37b08cec36b74c5 Mon Sep 17 00:00:00 2001 From: Manuel Stahl Date: Thu, 31 Oct 2019 13:17:07 +0100 Subject: [PATCH 01/11] Fix typo in handlers Signed-off-by: Manuel Stahl --- synapse/handlers/admin.py | 6 +++--- synapse/storage/data_stores/main/__init__.py | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/synapse/handlers/admin.py b/synapse/handlers/admin.py index 6407d56f8e7e..cc6d1947a969 100644 --- a/synapse/handlers/admin.py +++ b/synapse/handlers/admin.py @@ -56,7 +56,7 @@ def get_whois(self, user): @defer.inlineCallbacks def get_users(self): - """Function to reterive a list of users in users table. + """Function to retrieve a list of users in users table. Args: Returns: @@ -68,14 +68,14 @@ def get_users(self): @defer.inlineCallbacks def get_users_paginate(self, order, start, limit): - """Function to reterive a paginated list of users from + """Function to retrieve a paginated list of users from users list. This will return a json object, which contains list of users and the total number of users in users table. Args: order (str): column name to order the select by this column start (int): start number to begin the query from - limit (int): number of rows to reterive + limit (int): number of rows to retrieve Returns: defer.Deferred: resolves to json object {list[dict[str, Any]], count} """ diff --git a/synapse/storage/data_stores/main/__init__.py b/synapse/storage/data_stores/main/__init__.py index 10c940df1e2f..25a8805dbeaa 100644 --- a/synapse/storage/data_stores/main/__init__.py +++ b/synapse/storage/data_stores/main/__init__.py @@ -474,7 +474,7 @@ def _generate_user_daily_visits(txn): ) def get_users(self): - """Function to reterive a list of users in users table. + """Function to retrieve a list of users in users table. Args: Returns: @@ -489,7 +489,7 @@ def get_users(self): @defer.inlineCallbacks def get_users_paginate(self, order, start, limit): - """Function to reterive a paginated list of users from + """Function to retrieve a paginated list of users from users list. This will return a json object, which contains list of users and the total number of users in users table. From 4abc7383acaf270ad2110bb9f63e8b6a4b08be4f Mon Sep 17 00:00:00 2001 From: Manuel Stahl Date: Wed, 28 Aug 2019 16:13:08 +0200 Subject: [PATCH 02/11] Return deactivated property in admin users endpoint Signed-off-by: Manuel Stahl --- synapse/storage/data_stores/main/__init__.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/synapse/storage/data_stores/main/__init__.py b/synapse/storage/data_stores/main/__init__.py index 25a8805dbeaa..2e39f2c6e9e1 100644 --- a/synapse/storage/data_stores/main/__init__.py +++ b/synapse/storage/data_stores/main/__init__.py @@ -483,7 +483,14 @@ def get_users(self): return self._simple_select_list( table="users", keyvalues={}, - retcols=["name", "password_hash", "is_guest", "admin", "user_type"], + retcols=[ + "name", + "password_hash", + "is_guest", + "admin", + "user_type", + "deactivated", + ], desc="get_users", ) From 29fb2c1fc50c8155f40808a923c52cec2f992797 Mon Sep 17 00:00:00 2001 From: Manuel Stahl Date: Thu, 3 Oct 2019 18:08:09 +0200 Subject: [PATCH 03/11] Remove admin endpoint users_paginate Signed-off-by: Manuel Stahl --- changelog.d/5925.removal | 1 + synapse/rest/admin/__init__.py | 2 - synapse/rest/admin/users.py | 69 ---------------------------------- 3 files changed, 1 insertion(+), 71 deletions(-) create mode 100644 changelog.d/5925.removal diff --git a/changelog.d/5925.removal b/changelog.d/5925.removal new file mode 100644 index 000000000000..cbba2855cbc2 --- /dev/null +++ b/changelog.d/5925.removal @@ -0,0 +1 @@ +Remove admin/v1/users_paginate endpoint. Contributed by Awesome Technologies Innovationslabor GmbH. diff --git a/synapse/rest/admin/__init__.py b/synapse/rest/admin/__init__.py index 68a59a34249a..5b8eab71541e 100644 --- a/synapse/rest/admin/__init__.py +++ b/synapse/rest/admin/__init__.py @@ -34,7 +34,6 @@ from synapse.rest.admin.users import ( AccountValidityRenewServlet, DeactivateAccountRestServlet, - GetUsersPaginatedRestServlet, ResetPasswordRestServlet, SearchUsersRestServlet, UserAdminServlet, @@ -201,7 +200,6 @@ def register_servlets_for_client_rest_resource(hs, http_server): PurgeHistoryRestServlet(hs).register(http_server) UsersRestServlet(hs).register(http_server) ResetPasswordRestServlet(hs).register(http_server) - GetUsersPaginatedRestServlet(hs).register(http_server) SearchUsersRestServlet(hs).register(http_server) ShutdownRoomRestServlet(hs).register(http_server) UserRegisterServlet(hs).register(http_server) diff --git a/synapse/rest/admin/users.py b/synapse/rest/admin/users.py index 58a83f93af05..57f730be342a 100644 --- a/synapse/rest/admin/users.py +++ b/synapse/rest/admin/users.py @@ -25,7 +25,6 @@ from synapse.http.servlet import ( RestServlet, assert_params_in_dict, - parse_integer, parse_json_object_from_request, parse_string, ) @@ -59,74 +58,6 @@ async def on_GET(self, request, user_id): return 200, ret -class GetUsersPaginatedRestServlet(RestServlet): - """Get request to get specific number of users from Synapse. - This needs user to have administrator access in Synapse. - Example: - http://localhost:8008/_synapse/admin/v1/users_paginate/ - @admin:user?access_token=admin_access_token&start=0&limit=10 - Returns: - 200 OK with json object {list[dict[str, Any]], count} or empty object. - """ - - PATTERNS = historical_admin_path_patterns( - "/users_paginate/(?P[^/]*)" - ) - - def __init__(self, hs): - self.store = hs.get_datastore() - self.hs = hs - self.auth = hs.get_auth() - self.handlers = hs.get_handlers() - - async def on_GET(self, request, target_user_id): - """Get request to get specific number of users from Synapse. - This needs user to have administrator access in Synapse. - """ - await assert_requester_is_admin(self.auth, request) - - target_user = UserID.from_string(target_user_id) - - if not self.hs.is_mine(target_user): - raise SynapseError(400, "Can only users a local user") - - order = "name" # order by name in user table - start = parse_integer(request, "start", required=True) - limit = parse_integer(request, "limit", required=True) - - logger.info("limit: %s, start: %s", limit, start) - - ret = await self.handlers.admin_handler.get_users_paginate(order, start, limit) - return 200, ret - - async def on_POST(self, request, target_user_id): - """Post request to get specific number of users from Synapse.. - This needs user to have administrator access in Synapse. - Example: - http://localhost:8008/_synapse/admin/v1/users_paginate/ - @admin:user?access_token=admin_access_token - JsonBodyToSend: - { - "start": "0", - "limit": "10 - } - Returns: - 200 OK with json object {list[dict[str, Any]], count} or empty object. - """ - await assert_requester_is_admin(self.auth, request) - UserID.from_string(target_user_id) - - order = "name" # order by name in user table - params = parse_json_object_from_request(request) - assert_params_in_dict(params, ["limit", "start"]) - limit = params["limit"] - start = params["start"] - logger.info("limit: %s, start: %s", limit, start) - - ret = await self.handlers.admin_handler.get_users_paginate(order, start, limit) - return 200, ret - - class UserRegisterServlet(RestServlet): """ Attributes: From 18f680a2c99bf5a25d3d934d0d37a15087a0a13c Mon Sep 17 00:00:00 2001 From: Manuel Stahl Date: Thu, 31 Oct 2019 13:27:11 +0100 Subject: [PATCH 04/11] Add parameters "name", "guests" and "deactivated" to get_users_paginate Remove unused "order" parameter. Signed-off-by: Manuel Stahl --- synapse/handlers/admin.py | 15 +++--- synapse/storage/_base.py | 29 ++++++------ synapse/storage/data_stores/main/__init__.py | 49 +++++++++++++------- synapse/storage/data_stores/main/stats.py | 1 + 4 files changed, 55 insertions(+), 39 deletions(-) diff --git a/synapse/handlers/admin.py b/synapse/handlers/admin.py index cc6d1947a969..14449b9a1e66 100644 --- a/synapse/handlers/admin.py +++ b/synapse/handlers/admin.py @@ -67,19 +67,22 @@ def get_users(self): return ret @defer.inlineCallbacks - def get_users_paginate(self, order, start, limit): + def get_users_paginate(self, start, limit, name, guests, deactivated): """Function to retrieve a paginated list of users from - users list. This will return a json object, which contains - list of users and the total number of users in users table. + users list. This will return a json list of users. Args: - order (str): column name to order the select by this column start (int): start number to begin the query from limit (int): number of rows to retrieve + name (string): filter for user names + guests (bool): whether to in include guest users + deactivated (bool): whether to include deactivated users Returns: - defer.Deferred: resolves to json object {list[dict[str, Any]], count} + defer.Deferred: resolves to json list[dict[str, Any]] """ - ret = yield self.store.get_users_paginate(order, start, limit) + ret = yield self.store.get_users_paginate( + start, limit, name, guests, deactivated + ) return ret diff --git a/synapse/storage/_base.py b/synapse/storage/_base.py index 459901ac60a4..2d15b96769b3 100644 --- a/synapse/storage/_base.py +++ b/synapse/storage/_base.py @@ -1534,6 +1534,7 @@ def get_cache_stream_token(self): def _simple_select_list_paginate( self, table, + filters, keyvalues, orderby, start, @@ -1549,6 +1550,9 @@ def _simple_select_list_paginate( Args: table (str): the table name + 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. @@ -1564,6 +1568,7 @@ def _simple_select_list_paginate( desc, self._simple_select_list_paginate_txn, table, + filters, keyvalues, orderby, start, @@ -1577,6 +1582,7 @@ def _simple_select_list_paginate_txn( cls, txn, table, + filters, keyvalues, orderby, start, @@ -1592,6 +1598,9 @@ def _simple_select_list_paginate_txn( Args: txn : Transaction object table (str): the table name + 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. @@ -1606,10 +1615,12 @@ def _simple_select_list_paginate_txn( if order_direction not in ["ASC", "DESC"]: raise ValueError("order_direction must be one of 'ASC' or 'DESC'.") + where_clause = "WHERE " if filters or keyvalues else "" + if filters: + where_clause += " AND ".join("%s LIKE ?" % (k,) for k in filters) + where_clause += " AND " if filters and keyvalues else "" if keyvalues: - where_clause = "WHERE " + " AND ".join("%s = ?" % (k,) for k in keyvalues) - else: - where_clause = "" + where_clause += " AND ".join("%s = ?" % (k,) for k in keyvalues) sql = "SELECT %s FROM %s %s ORDER BY %s %s LIMIT ? OFFSET ?" % ( ", ".join(retcols), @@ -1622,18 +1633,6 @@ def _simple_select_list_paginate_txn( return cls.cursor_to_dict(txn) - def get_user_count_txn(self, txn): - """Get a total number of registered users in the users list. - - Args: - txn : Transaction object - Returns: - int : number of users - """ - sql_count = "SELECT COUNT(*) FROM users WHERE is_guest = 0;" - txn.execute(sql_count) - return txn.fetchone()[0] - def _simple_search_list( self, table, term, col, retcols, desc="_simple_search_list" ): diff --git a/synapse/storage/data_stores/main/__init__.py b/synapse/storage/data_stores/main/__init__.py index 2e39f2c6e9e1..642711207864 100644 --- a/synapse/storage/data_stores/main/__init__.py +++ b/synapse/storage/data_stores/main/__init__.py @@ -19,8 +19,6 @@ import logging import time -from twisted.internet import defer - from synapse.api.constants import PresenceState from synapse.storage.engines import PostgresEngine from synapse.storage.util.id_generators import ( @@ -494,32 +492,47 @@ def get_users(self): desc="get_users", ) - @defer.inlineCallbacks - def get_users_paginate(self, order, start, limit): + def get_users_paginate( + self, start, limit, name=None, guests=True, deactivated=False + ): """Function to retrieve a paginated list of users from - users list. This will return a json object, which contains - list of users and the total number of users in users table. + users list. This will return a json list of users. Args: - order (str): column name to order the select by this column start (int): start number to begin the query from - limit (int): number of rows to reterive + limit (int): number of rows to retrieve + name (string): filter for user names + guests (bool): whether to in include guest users + deactivated (bool): whether to include deactivated users Returns: - defer.Deferred: resolves to json object {list[dict[str, Any]], count} + defer.Deferred: resolves to list[dict[str, Any]] """ - users = yield self.runInteraction( - "get_users_paginate", - self._simple_select_list_paginate_txn, + name_filter = {} + if name: + name_filter["name"] = name + + attr_filter = {} + if not guests: + attr_filter["is_guest"] = False + if not deactivated: + attr_filter["deactivated"] = False + + return self._simple_select_list_paginate( table="users", - keyvalues={"is_guest": False}, - orderby=order, + filters=name_filter, + keyvalues=attr_filter, + orderby="name", start=start, limit=limit, - retcols=["name", "password_hash", "is_guest", "admin", "user_type"], + retcols=[ + "name", + "password_hash", + "is_guest", + "admin", + "user_type", + "deactivated", + ], ) - count = yield self.runInteraction("get_users_paginate", self.get_user_count_txn) - retval = {"users": users, "total": count} - return retval def search_users(self, term): """Function to search users list for one or more users with diff --git a/synapse/storage/data_stores/main/stats.py b/synapse/storage/data_stores/main/stats.py index 45b3de7d56c4..b7bcd89ce9c6 100644 --- a/synapse/storage/data_stores/main/stats.py +++ b/synapse/storage/data_stores/main/stats.py @@ -260,6 +260,7 @@ 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, From cbc6dd373cb3b0cf96f466abfa1b064978bae117 Mon Sep 17 00:00:00 2001 From: Manuel Stahl Date: Thu, 3 Oct 2019 19:15:11 +0200 Subject: [PATCH 05/11] Add admin/v2/users endpoint with pagination Signed-off-by: Manuel Stahl --- changelog.d/5925.feature | 1 + docs/admin_api/user_admin_api.rst | 43 +++++++++++++++++++++++++++++++ synapse/rest/admin/__init__.py | 2 ++ synapse/rest/admin/users.py | 41 +++++++++++++++++++++++++++++ 4 files changed, 87 insertions(+) create mode 100644 changelog.d/5925.feature diff --git a/changelog.d/5925.feature b/changelog.d/5925.feature new file mode 100644 index 000000000000..8025cc82315b --- /dev/null +++ b/changelog.d/5925.feature @@ -0,0 +1 @@ +Add admin/v2/users endpoint with pagination. Contributed by Awesome Technologies Innovationslabor GmbH. diff --git a/docs/admin_api/user_admin_api.rst b/docs/admin_api/user_admin_api.rst index d0871f943844..a709e10f4e77 100644 --- a/docs/admin_api/user_admin_api.rst +++ b/docs/admin_api/user_admin_api.rst @@ -1,3 +1,46 @@ +List Accounts +============= + +This API returns all local user accounts. + +The api is:: + + GET /_synapse/admin/v2/users?offset=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. + +It returns a JSON body like the following: + +.. code:: json + + { + "users": [ + { + "name": "", + "password_hash": "", + "is_guest": 0, + "admin": 0, + "user_type": null, + "deactivated": 0 + }, { + "name": "", + "password_hash": "", + "is_guest": 0, + "admin": 1, + "user_type": null, + "deactivated": 0 + } + ], + "next_token": 100 + } + + Query Account ============= diff --git a/synapse/rest/admin/__init__.py b/synapse/rest/admin/__init__.py index 5b8eab71541e..c122c449f401 100644 --- a/synapse/rest/admin/__init__.py +++ b/synapse/rest/admin/__init__.py @@ -39,6 +39,7 @@ UserAdminServlet, UserRegisterServlet, UsersRestServlet, + UsersRestServletV2, WhoisRestServlet, ) from synapse.util.versionstring import get_version_string @@ -190,6 +191,7 @@ def register_servlets(hs, http_server): SendServerNoticeServlet(hs).register(http_server) VersionServlet(hs).register(http_server) UserAdminServlet(hs).register(http_server) + UsersRestServletV2(hs).register(http_server) def register_servlets_for_client_rest_resource(hs, http_server): diff --git a/synapse/rest/admin/users.py b/synapse/rest/admin/users.py index 57f730be342a..c43f7001ad10 100644 --- a/synapse/rest/admin/users.py +++ b/synapse/rest/admin/users.py @@ -25,6 +25,8 @@ from synapse.http.servlet import ( RestServlet, assert_params_in_dict, + parse_boolean, + parse_integer, parse_json_object_from_request, parse_string, ) @@ -58,6 +60,45 @@ async def on_GET(self, request, user_id): return 200, ret +class UsersRestServletV2(RestServlet): + PATTERNS = (re.compile("^/_synapse/admin/v2/users$"),) + + """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 + + 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 parameter `guests` can be used to exclude guest users. + The parameter `deactivated` can be used to include deactivated users. + """ + + def __init__(self, hs): + self.hs = hs + self.auth = hs.get_auth() + self.admin_handler = hs.get_handlers().admin_handler + + async def on_GET(self, request): + await assert_requester_is_admin(self.auth, request) + + start = parse_integer(request, "offset", default=0) + limit = parse_integer(request, "limit", default=100) + name = parse_string(request, "name", 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 + ) + return 200, {"users": users, "next_token": start + len(users)} + + class UserRegisterServlet(RestServlet): """ Attributes: From ac8757d75d680f70744ab45c75be83b9d3d1c96c Mon Sep 17 00:00:00 2001 From: Manuel Stahl Date: Wed, 4 Dec 2019 15:58:45 +0100 Subject: [PATCH 06/11] Fix review comments --- docs/admin_api/user_admin_api.rst | 23 +++++++++-------- synapse/rest/admin/users.py | 21 ++++++++------- synapse/storage/_base.py | 27 ++++++++++++-------- synapse/storage/data_stores/main/__init__.py | 4 +-- synapse/storage/data_stores/main/stats.py | 3 +-- 5 files changed, 43 insertions(+), 35 deletions(-) diff --git a/docs/admin_api/user_admin_api.rst b/docs/admin_api/user_admin_api.rst index a709e10f4e77..9d52d4097044 100644 --- a/docs/admin_api/user_admin_api.rst +++ b/docs/admin_api/user_admin_api.rst @@ -5,16 +5,17 @@ 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 @@ -22,14 +23,14 @@ It returns a JSON body like the following: { "users": [ { - "name": "", + "user_id": "", "password_hash": "", "is_guest": 0, "admin": 0, "user_type": null, "deactivated": 0 }, { - "name": "", + "user_id": "", "password_hash": "", "is_guest": 0, "admin": 1, @@ -37,7 +38,7 @@ It returns a JSON body like the following: "deactivated": 0 } ], - "next_token": 100 + "next_token": "100" } diff --git a/synapse/rest/admin/users.py b/synapse/rest/admin/users.py index c43f7001ad10..1937879dbe17 100644 --- a/synapse/rest/admin/users.py +++ b/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): diff --git a/synapse/storage/_base.py b/synapse/storage/_base.py index 2d15b96769b3..c6d6aee29124 100644 --- a/synapse/storage/_base.py +++ b/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) diff --git a/synapse/storage/data_stores/main/__init__.py b/synapse/storage/data_stores/main/__init__.py index 642711207864..9e44b7593fd9 100644 --- a/synapse/storage/data_stores/main/__init__.py +++ b/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", diff --git a/synapse/storage/data_stores/main/stats.py b/synapse/storage/data_stores/main/stats.py index b7bcd89ce9c6..ef83c3dcb214 100644 --- a/synapse/storage/data_stores/main/stats.py +++ b/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", ) From d0f4dfdc1d3e28926370886275418090f5082e55 Mon Sep 17 00:00:00 2001 From: Manuel Stahl Date: Wed, 4 Dec 2019 16:18:39 +0100 Subject: [PATCH 07/11] Fix runtime error --- synapse/storage/_base.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/synapse/storage/_base.py b/synapse/storage/_base.py index c6d6aee29124..47f37b714b92 100644 --- a/synapse/storage/_base.py +++ b/synapse/storage/_base.py @@ -1619,11 +1619,14 @@ def _simple_select_list_paginate_txn( raise ValueError("order_direction must be one of 'ASC' or 'DESC'.") where_clause = "WHERE " if filters or keyvalues else "" + arg_list = [] if filters: where_clause += " AND ".join("%s LIKE ?" % (k,) for k in filters) + arg_list += list(filters.values()) where_clause += " AND " if filters and keyvalues else "" if keyvalues: where_clause += " AND ".join("%s = ?" % (k,) for k in keyvalues) + arg_list += list(keyvalues.values()) sql = "SELECT %s FROM %s %s ORDER BY %s %s LIMIT ? OFFSET ?" % ( ", ".join(retcols), @@ -1632,9 +1635,7 @@ def _simple_select_list_paginate_txn( orderby, order_direction, ) - txn.execute( - sql, list(filters.values()) + list(keyvalues.values()) + [limit, start] - ) + txn.execute(sql, arg_list + [limit, start]) return cls.cursor_to_dict(txn) From b9f6992eac950edffcc53503f0f2ed2821fb82b6 Mon Sep 17 00:00:00 2001 From: Manuel Stahl Date: Wed, 4 Dec 2019 17:55:50 +0100 Subject: [PATCH 08/11] Fix comments --- docs/admin_api/user_admin_api.rst | 7 ++++--- synapse/storage/_base.py | 4 ++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/docs/admin_api/user_admin_api.rst b/docs/admin_api/user_admin_api.rst index 9d52d4097044..b451dc501401 100644 --- a/docs/admin_api/user_admin_api.rst +++ b/docs/admin_api/user_admin_api.rst @@ -10,7 +10,8 @@ The api is:: including an ``access_token`` of a server admin. 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 ``user_id`` can be used to select only users with user ids that +contain this value. 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, @@ -23,14 +24,14 @@ It returns a JSON body like the following: { "users": [ { - "user_id": "", + "name": "", "password_hash": "", "is_guest": 0, "admin": 0, "user_type": null, "deactivated": 0 }, { - "user_id": "", + "name": "", "password_hash": "", "is_guest": 0, "admin": 1, diff --git a/synapse/storage/_base.py b/synapse/storage/_base.py index 47f37b714b92..2ecc73b023df 100644 --- a/synapse/storage/_base.py +++ b/synapse/storage/_base.py @@ -1594,8 +1594,8 @@ def _simple_select_list_paginate_txn( 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 + Use `filters` to search attributes using SQL wildcards and/or `keyvalues` to + select attributes with exact matches. All constraints are joined together using 'AND'. Args: From 0df165d47781d0b1e244f4bf4e661efdc234924b Mon Sep 17 00:00:00 2001 From: Manuel Stahl Date: Wed, 4 Dec 2019 18:45:31 +0100 Subject: [PATCH 09/11] Fix search expression for users --- synapse/storage/data_stores/main/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/storage/data_stores/main/__init__.py b/synapse/storage/data_stores/main/__init__.py index 9e44b7593fd9..1b009aa01d7e 100644 --- a/synapse/storage/data_stores/main/__init__.py +++ b/synapse/storage/data_stores/main/__init__.py @@ -509,7 +509,7 @@ def get_users_paginate( """ name_filter = {} if name: - name_filter["name"] = name + name_filter["name"] = "%%" + name + "%%" attr_filter = {} if not guests: From 147a9a91d35423ccf76fc1789dce588d6c951dd2 Mon Sep 17 00:00:00 2001 From: Manuel Stahl Date: Wed, 4 Dec 2019 19:17:14 +0100 Subject: [PATCH 10/11] Use single quotes instead of double quotes --- synapse/storage/data_stores/main/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/storage/data_stores/main/__init__.py b/synapse/storage/data_stores/main/__init__.py index 1b009aa01d7e..7c10d6280db2 100644 --- a/synapse/storage/data_stores/main/__init__.py +++ b/synapse/storage/data_stores/main/__init__.py @@ -509,7 +509,7 @@ def get_users_paginate( """ name_filter = {} if name: - name_filter["name"] = "%%" + name + "%%" + name_filter["name"] = "%" + name + "%" attr_filter = {} if not guests: From 89925acdd710f0d3322cabd533f19ee5b002895d Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Thu, 5 Dec 2019 17:26:04 +0000 Subject: [PATCH 11/11] fix lint --- synapse/storage/_base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/storage/_base.py b/synapse/storage/_base.py index 05bff5ea10c0..0d7c7dff273c 100644 --- a/synapse/storage/_base.py +++ b/synapse/storage/_base.py @@ -1410,7 +1410,7 @@ def simple_select_list_paginate_txn( 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 wildcards and/or `keyvalues` to select attributes with exact matches. All constraints are joined together using 'AND'.