From 26de7a1d47d55bc8ebe150177223d26e11b7469c Mon Sep 17 00:00:00 2001 From: Manuel Stahl Date: Mon, 10 Feb 2020 10:20:36 +0100 Subject: [PATCH 1/3] Fix admin API for users when "avatar_url" is "null" Signed-off-by: Manuel Stahl --- synapse/rest/admin/users.py | 4 ++-- tests/rest/admin/test_user.py | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/synapse/rest/admin/users.py b/synapse/rest/admin/users.py index e75c5f137028..8f42d436cba1 100644 --- a/synapse/rest/admin/users.py +++ b/synapse/rest/admin/users.py @@ -196,7 +196,7 @@ async def on_PUT(self, request, user_id): user_id, threepid["medium"], threepid["address"], current_time ) - if "avatar_url" in body: + if "avatar_url" in body and type(body["avatar_url"]) == str: await self.profile_handler.set_avatar_url( target_user, requester, body["avatar_url"], True ) @@ -271,7 +271,7 @@ async def on_PUT(self, request, user_id): user_id, threepid["medium"], threepid["address"], current_time ) - if "avatar_url" in body: + if "avatar_url" in body and type(body["avatar_url"]) == str: await self.profile_handler.set_avatar_url( user_id, requester, body["avatar_url"], True ) diff --git a/tests/rest/admin/test_user.py b/tests/rest/admin/test_user.py index 3b5169b38d25..6525cbf2e515 100644 --- a/tests/rest/admin/test_user.py +++ b/tests/rest/admin/test_user.py @@ -412,6 +412,7 @@ def test_requester_is_admin(self): "password": "abc123", "admin": True, "threepids": [{"medium": "email", "address": "bob@bob.bob"}], + "avatar_url": None, } ) From d04b19cc070ea79244be1c1990e9e1d20a14c47d Mon Sep 17 00:00:00 2001 From: Manuel Stahl Date: Mon, 10 Feb 2020 12:47:18 +0100 Subject: [PATCH 2/3] Return total number of users and profile attributes in admin users endpoint Signed-off-by: Manuel Stahl --- changelog.d/6881.misc | 1 + docs/admin_api/user_admin_api.rst | 11 +++- synapse/rest/admin/users.py | 4 +- synapse/storage/data_stores/main/__init__.py | 68 +++++++++++--------- tests/rest/admin/test_user.py | 1 + tests/storage/test_main.py | 46 +++++++++++++ 6 files changed, 97 insertions(+), 34 deletions(-) create mode 100644 changelog.d/6881.misc create mode 100644 tests/storage/test_main.py diff --git a/changelog.d/6881.misc b/changelog.d/6881.misc new file mode 100644 index 000000000000..03b89ccd3d87 --- /dev/null +++ b/changelog.d/6881.misc @@ -0,0 +1 @@ +Return total number of users and profile attributes in admin users endpoint. 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 eb146095decb..08528186bd89 100644 --- a/docs/admin_api/user_admin_api.rst +++ b/docs/admin_api/user_admin_api.rst @@ -70,17 +70,22 @@ It returns a JSON body like the following: "is_guest": 0, "admin": 0, "user_type": null, - "deactivated": 0 + "deactivated": 0, + "displayname": , + "avatar_url": null }, { "name": "", "password_hash": "", "is_guest": 0, "admin": 1, "user_type": null, - "deactivated": 0 + "deactivated": 0, + "displayname": , + "avatar_url": "https://example.com/user_id2" } ], - "next_token": "100" + "next_token": "100", + "total": 200 } diff --git a/synapse/rest/admin/users.py b/synapse/rest/admin/users.py index 8f42d436cba1..81da65e5dc38 100644 --- a/synapse/rest/admin/users.py +++ b/synapse/rest/admin/users.py @@ -94,10 +94,10 @@ async def on_GET(self, request): guests = parse_boolean(request, "guests", default=True) deactivated = parse_boolean(request, "deactivated", default=False) - users = await self.store.get_users_paginate( + users, total = await self.store.get_users_paginate( start, limit, user_id, guests, deactivated ) - ret = {"users": users} + ret = {"users": users, "total": total} if len(users) >= limit: ret["next_token"] = str(start + len(users)) diff --git a/synapse/storage/data_stores/main/__init__.py b/synapse/storage/data_stores/main/__init__.py index 2700cca822f6..ab1d148b6d97 100644 --- a/synapse/storage/data_stores/main/__init__.py +++ b/synapse/storage/data_stores/main/__init__.py @@ -509,7 +509,8 @@ 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 list of users. + users list. This will return a json list of users and the + total number of users matching the filter criteria. Args: start (int): start number to begin the query from @@ -518,35 +519,44 @@ def get_users_paginate( guests (bool): whether to in include guest users deactivated (bool): whether to include deactivated users Returns: - defer.Deferred: resolves to list[dict[str, Any]] + defer.Deferred: resolves to list[dict[str, Any]], int """ - name_filter = {} - if name: - name_filter["name"] = "%" + name + "%" - - attr_filter = {} - if not guests: - attr_filter["is_guest"] = 0 - if not deactivated: - attr_filter["deactivated"] = 0 - - return self.db.simple_select_list_paginate( - desc="get_users_paginate", - table="users", - orderby="name", - start=start, - limit=limit, - filters=name_filter, - keyvalues=attr_filter, - retcols=[ - "name", - "password_hash", - "is_guest", - "admin", - "user_type", - "deactivated", - ], - ) + + def get_users_paginate_txn(txn): + filters = [] + args = [] + + if name: + filters.append("name LIKE ?") + args.append("%" + name + "%") + + if not guests: + filters.append("is_guest = 0") + + if not deactivated: + filters.append("deactivated = 0") + + where_clause = "WHERE " + " AND ".join(filters) if len(filters) > 0 else "" + + sql = "SELECT COUNT(*) as total_users FROM users %s" % (where_clause) + txn.execute(sql, args) + count = txn.fetchone()[0] + + args = [self.hs.config.server_name] + args + [limit, start] + sql = """ + SELECT name, user_type, is_guest, admin, deactivated, displayname, avatar_url + FROM users as u + LEFT JOIN profiles AS p ON u.name = '@' || p.user_id || ':' || ? + {} + ORDER BY u.name LIMIT ? OFFSET ? + """.format( + where_clause + ) + txn.execute(sql, args) + users = self.db.cursor_to_dict(txn) + return users, count + + return self.db.runInteraction("get_users_paginate_txn", get_users_paginate_txn) def search_users(self, term): """Function to search users list for one or more users with diff --git a/tests/rest/admin/test_user.py b/tests/rest/admin/test_user.py index 6525cbf2e515..6951b5a90fc1 100644 --- a/tests/rest/admin/test_user.py +++ b/tests/rest/admin/test_user.py @@ -359,6 +359,7 @@ def test_all_users(self): self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) self.assertEqual(3, len(channel.json_body["users"])) + self.assertEqual(3, channel.json_body["total"]) class UserRestTestCase(unittest.HomeserverTestCase): diff --git a/tests/storage/test_main.py b/tests/storage/test_main.py new file mode 100644 index 000000000000..ab0df5ea934a --- /dev/null +++ b/tests/storage/test_main.py @@ -0,0 +1,46 @@ +# -*- coding: utf-8 -*- +# Copyright 2020 Awesome Technologies Innovationslabor GmbH +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + + +from twisted.internet import defer + +from synapse.types import UserID + +from tests import unittest +from tests.utils import setup_test_homeserver + + +class DataStoreTestCase(unittest.TestCase): + @defer.inlineCallbacks + def setUp(self): + hs = yield setup_test_homeserver(self.addCleanup) + + self.store = hs.get_datastore() + + self.user = UserID.from_string("@abcde:test") + self.displayname = "Frank" + + @defer.inlineCallbacks + def test_get_users_paginate(self): + yield self.store.register_user(self.user.to_string(), "pass") + yield self.store.create_profile(self.user.localpart) + yield self.store.set_profile_displayname(self.user.localpart, self.displayname) + + users, total = yield self.store.get_users_paginate( + 0, 10, name="bc", guests=False + ) + + self.assertEquals(1, total) + self.assertEquals(self.displayname, users.pop()["displayname"]) From 3d4c97fd15987a68082cf2bfb164cd0a18153fa8 Mon Sep 17 00:00:00 2001 From: Manuel Stahl Date: Tue, 28 Apr 2020 18:46:10 +0200 Subject: [PATCH 3/3] Fix avatar_url in user_admin_api Use same placeholder as for "Create or modify Account" example. --- docs/admin_api/user_admin_api.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/admin_api/user_admin_api.rst b/docs/admin_api/user_admin_api.rst index 08528186bd89..092774fc9f91 100644 --- a/docs/admin_api/user_admin_api.rst +++ b/docs/admin_api/user_admin_api.rst @@ -81,7 +81,7 @@ It returns a JSON body like the following: "user_type": null, "deactivated": 0, "displayname": , - "avatar_url": "https://example.com/user_id2" + "avatar_url": "" } ], "next_token": "100",