From 7fe7b42e4b3f7c9b0172bc79dd84deafc94a23b7 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Thu, 17 Mar 2022 17:42:30 +0000 Subject: [PATCH 1/8] Formally type the UserProfile in user searches --- synapse/events/spamcheck.py | 7 +++--- synapse/handlers/user_directory.py | 4 ++-- synapse/rest/client/user_directory.py | 4 ++-- .../storage/databases/main/user_directory.py | 22 +++++++++++++++---- synapse/types.py | 11 ++++++++++ 5 files changed, 36 insertions(+), 12 deletions(-) diff --git a/synapse/events/spamcheck.py b/synapse/events/spamcheck.py index 60904a55f5c0..cd80fcf9d13a 100644 --- a/synapse/events/spamcheck.py +++ b/synapse/events/spamcheck.py @@ -21,7 +21,6 @@ Awaitable, Callable, Collection, - Dict, List, Optional, Tuple, @@ -31,7 +30,7 @@ from synapse.rest.media.v1._base import FileInfo from synapse.rest.media.v1.media_storage import ReadableFileWrapper from synapse.spam_checker_api import RegistrationBehaviour -from synapse.types import RoomAlias +from synapse.types import RoomAlias, UserProfile from synapse.util.async_helpers import maybe_awaitable if TYPE_CHECKING: @@ -50,7 +49,7 @@ USER_MAY_CREATE_ROOM_CALLBACK = Callable[[str], Awaitable[bool]] USER_MAY_CREATE_ROOM_ALIAS_CALLBACK = Callable[[str, RoomAlias], Awaitable[bool]] USER_MAY_PUBLISH_ROOM_CALLBACK = Callable[[str, str], Awaitable[bool]] -CHECK_USERNAME_FOR_SPAM_CALLBACK = Callable[[Dict[str, str]], Awaitable[bool]] +CHECK_USERNAME_FOR_SPAM_CALLBACK = Callable[[UserProfile], Awaitable[bool]] LEGACY_CHECK_REGISTRATION_FOR_SPAM_CALLBACK = Callable[ [ Optional[dict], @@ -383,7 +382,7 @@ async def user_may_publish_room(self, userid: str, room_id: str) -> bool: return True - async def check_username_for_spam(self, user_profile: Dict[str, str]) -> bool: + async def check_username_for_spam(self, user_profile: UserProfile) -> bool: """Checks if a user ID or display name are considered "spammy" by this server. If the server considers a username spammy, then it will not be included in diff --git a/synapse/handlers/user_directory.py b/synapse/handlers/user_directory.py index d27ed2be6a99..048fd4bb8225 100644 --- a/synapse/handlers/user_directory.py +++ b/synapse/handlers/user_directory.py @@ -19,8 +19,8 @@ from synapse.api.constants import EventTypes, HistoryVisibility, JoinRules, Membership from synapse.handlers.state_deltas import MatchChange, StateDeltasHandler from synapse.metrics.background_process_metrics import run_as_background_process +from synapse.storage.databases.main.user_directory import SearchResult from synapse.storage.roommember import ProfileInfo -from synapse.types import JsonDict from synapse.util.metrics import Measure if TYPE_CHECKING: @@ -78,7 +78,7 @@ def __init__(self, hs: "HomeServer"): async def search_users( self, user_id: str, search_term: str, limit: int - ) -> JsonDict: + ) -> SearchResult: """Searches for users in directory Returns: diff --git a/synapse/rest/client/user_directory.py b/synapse/rest/client/user_directory.py index a47d9bd01da5..116c982ce637 100644 --- a/synapse/rest/client/user_directory.py +++ b/synapse/rest/client/user_directory.py @@ -19,7 +19,7 @@ from synapse.http.server import HttpServer from synapse.http.servlet import RestServlet, parse_json_object_from_request from synapse.http.site import SynapseRequest -from synapse.types import JsonDict +from synapse.types import JsonMapping from ._base import client_patterns @@ -38,7 +38,7 @@ def __init__(self, hs: "HomeServer"): self.auth = hs.get_auth() self.user_directory_handler = hs.get_user_directory_handler() - async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]: + async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonMapping]: """Searches for users in directory Returns: diff --git a/synapse/storage/databases/main/user_directory.py b/synapse/storage/databases/main/user_directory.py index e7fddd24262a..4bff0ea17b9b 100644 --- a/synapse/storage/databases/main/user_directory.py +++ b/synapse/storage/databases/main/user_directory.py @@ -23,6 +23,7 @@ Sequence, Set, Tuple, + TypedDict, cast, ) @@ -40,7 +41,12 @@ from synapse.storage.databases.main.state import StateFilter from synapse.storage.databases.main.state_deltas import StateDeltasStore from synapse.storage.engines import PostgresEngine, Sqlite3Engine -from synapse.types import JsonDict, get_domain_from_id, get_localpart_from_id +from synapse.types import ( + JsonDict, + UserProfile, + get_domain_from_id, + get_localpart_from_id, +) from synapse.util.caches.descriptors import cached logger = logging.getLogger(__name__) @@ -591,6 +597,11 @@ async def update_user_directory_stream_pos(self, stream_id: Optional[int]) -> No ) +class SearchResult(TypedDict): + limited: bool + results: List[UserProfile] + + class UserDirectoryStore(UserDirectoryBackgroundUpdateStore): # How many records do we calculate before sending it to # add_users_who_share_private_rooms? @@ -777,7 +788,7 @@ async def get_user_directory_stream_pos(self) -> Optional[int]: async def search_user_dir( self, user_id: str, search_term: str, limit: int - ) -> JsonDict: + ) -> SearchResult: """Searches for users in directory Returns: @@ -910,8 +921,11 @@ async def search_user_dir( # This should be unreachable. raise Exception("Unrecognized database engine") - results = await self.db_pool.execute( - "search_user_dir", self.db_pool.cursor_to_dict, sql, *args + results = cast( + List[UserProfile], + await self.db_pool.execute( + "search_user_dir", self.db_pool.cursor_to_dict, sql, *args + ), ) limited = len(results) > limit diff --git a/synapse/types.py b/synapse/types.py index 53be3583a013..1df385924036 100644 --- a/synapse/types.py +++ b/synapse/types.py @@ -27,6 +27,7 @@ Optional, Tuple, Type, + TypedDict, TypeVar, Union, ) @@ -63,6 +64,10 @@ # JSON types. These could be made stronger, but will do for now. # A JSON-serialisable dict. JsonDict = Dict[str, Any] +# A JSON-serialisable mapping; roughly speaking an immutable JSONDict. +# Useful when you have a TypedDict which isn't going to be mutated and you don't want +# to cast to JsonDict everywhere. +JsonMapping = Mapping[str, Any] # A JSON-serialisable object. JsonSerializable = object @@ -791,3 +796,9 @@ class UserInfo: is_deactivated: bool is_guest: bool is_shadow_banned: bool + + +class UserProfile(TypedDict): + user_id: str + display_name: Optional[str] + avatar_url: Optional[str] From c391880dd32c7232f9b2a0db905349afa27d796f Mon Sep 17 00:00:00 2001 From: David Robertson Date: Thu, 17 Mar 2022 17:51:25 +0000 Subject: [PATCH 2/8] Update docs --- docs/modules/spam_checker_callbacks.md | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/docs/modules/spam_checker_callbacks.md b/docs/modules/spam_checker_callbacks.md index 2b672b78f9ae..f6ff5528af3a 100644 --- a/docs/modules/spam_checker_callbacks.md +++ b/docs/modules/spam_checker_callbacks.md @@ -172,7 +172,7 @@ any of the subsequent implementations of this callback. _First introduced in Synapse v1.37.0_ ```python -async def check_username_for_spam(user_profile: Dict[str, str]) -> bool +async def check_username_for_spam(user_profile: synapse.types.UserProfile) -> bool ``` Called when computing search results in the user directory. The module must return a @@ -180,11 +180,13 @@ Called when computing search results in the user directory. The module must retu searches. Return `True` to indicate that the user is spammy and exclude them from search results; otherwise return `False`. -The profile is represented as a dictionary with the following keys: +The profile is represented as a dictionary with the following keys. -* `user_id`: The Matrix ID for this user. -* `display_name`: The user's display name. -* `avatar_url`: The `mxc://` URL to the user's avatar. +* `user_id: str`. The Matrix ID for this user. +* `display_name: Optional[str]`. The user's display name, or `None` if this user + has not set a display name. +* `avatar_url: Optional[str]`. The `mxc://` URL to the user's avatar, or `None` + if this user has not set an avatar. The module is given a copy of the original dictionary, so modifying it from within the module cannot modify a user's profile when included in user directory search results. From ba3856fd8165bb3fc5299a230e4324a62d086af4 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Thu, 17 Mar 2022 17:57:11 +0000 Subject: [PATCH 3/8] Changelog --- changelog.d/12246.doc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/12246.doc diff --git a/changelog.d/12246.doc b/changelog.d/12246.doc new file mode 100644 index 000000000000..e7fcc1b99c78 --- /dev/null +++ b/changelog.d/12246.doc @@ -0,0 +1 @@ +Correct `check_username_for_spam` annotations and docs. \ No newline at end of file From 4a65178d099b2c5e01460cdb0376a3cd53f780c0 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Fri, 18 Mar 2022 12:44:54 +0000 Subject: [PATCH 4/8] Use Sean's colon Co-authored-by: Sean Quah <8349537+squahtx@users.noreply.github.com> --- docs/modules/spam_checker_callbacks.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/modules/spam_checker_callbacks.md b/docs/modules/spam_checker_callbacks.md index f6ff5528af3a..50cef6e2f950 100644 --- a/docs/modules/spam_checker_callbacks.md +++ b/docs/modules/spam_checker_callbacks.md @@ -180,7 +180,7 @@ Called when computing search results in the user directory. The module must retu searches. Return `True` to indicate that the user is spammy and exclude them from search results; otherwise return `False`. -The profile is represented as a dictionary with the following keys. +The profile is represented as a dictionary with the following keys: * `user_id: str`. The Matrix ID for this user. * `display_name: Optional[str]`. The user's display name, or `None` if this user From d54b35bab8cf74726e9b816ea1067f90734a8fc6 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Fri, 18 Mar 2022 12:53:56 +0000 Subject: [PATCH 5/8] import TypedDict from typing extensions I'm out of practice. --- synapse/storage/databases/main/user_directory.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/synapse/storage/databases/main/user_directory.py b/synapse/storage/databases/main/user_directory.py index 4bff0ea17b9b..55cc9178f0b8 100644 --- a/synapse/storage/databases/main/user_directory.py +++ b/synapse/storage/databases/main/user_directory.py @@ -23,10 +23,11 @@ Sequence, Set, Tuple, - TypedDict, cast, ) +from typing_extensions import TypedDict + from synapse.api.errors import StoreError if TYPE_CHECKING: From 8a1eae8f23c474a953223c62cd92203f2f25f0df Mon Sep 17 00:00:00 2001 From: David Robertson Date: Fri, 18 Mar 2022 12:58:39 +0000 Subject: [PATCH 6/8] export UserProfile in synapse.module_api --- docs/modules/spam_checker_callbacks.md | 2 +- synapse/module_api/__init__.py | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/modules/spam_checker_callbacks.md b/docs/modules/spam_checker_callbacks.md index 50cef6e2f950..472d95718087 100644 --- a/docs/modules/spam_checker_callbacks.md +++ b/docs/modules/spam_checker_callbacks.md @@ -172,7 +172,7 @@ any of the subsequent implementations of this callback. _First introduced in Synapse v1.37.0_ ```python -async def check_username_for_spam(user_profile: synapse.types.UserProfile) -> bool +async def check_username_for_spam(user_profile: synapse.module_api.UserProfile) -> bool ``` Called when computing search results in the user directory. The module must return a diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index d735c1d4616e..aa8256b36f1d 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -111,6 +111,7 @@ StateMap, UserID, UserInfo, + UserProfile, create_requester, ) from synapse.util import Clock @@ -150,6 +151,7 @@ "EventBase", "StateMap", "ProfileInfo", + "UserProfile", ] logger = logging.getLogger(__name__) From ada0b329f5ab3cb4ddadfcd3bddb9f969f540195 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Fri, 18 Mar 2022 13:18:36 +0000 Subject: [PATCH 7/8] import TypedDict from typing_extensions, part 2 --- synapse/types.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/synapse/types.py b/synapse/types.py index 1df385924036..3c93243fdca1 100644 --- a/synapse/types.py +++ b/synapse/types.py @@ -27,11 +27,12 @@ Optional, Tuple, Type, - TypedDict, TypeVar, Union, ) +from typing_extensions import TypedDict + import attr from frozendict import frozendict from signedjson.key import decode_verify_key_bytes From 5747d6957e6b98135d9a103182ef4d2a19b6d196 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Fri, 18 Mar 2022 13:21:40 +0000 Subject: [PATCH 8/8] linter script --- synapse/types.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/synapse/types.py b/synapse/types.py index 3c93243fdca1..5ce2a5b0a5ee 100644 --- a/synapse/types.py +++ b/synapse/types.py @@ -31,11 +31,10 @@ Union, ) -from typing_extensions import TypedDict - import attr from frozendict import frozendict from signedjson.key import decode_verify_key_bytes +from typing_extensions import TypedDict from unpaddedbase64 import decode_base64 from zope.interface import Interface