From 3a4342db671d0766ab60f8f4f98df9e117d823a2 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 31 Dec 2024 14:20:25 -0500 Subject: [PATCH 1/3] Refactor get_profile: do not return missing fields. --- changelog.d/18063.misc | 1 + synapse/api/constants.py | 5 +++++ synapse/handlers/profile.py | 14 +++++++++----- synapse/handlers/sso.py | 9 +++++---- synapse/handlers/user_directory.py | 16 +++++++++++++--- synapse/module_api/__init__.py | 18 ++++++++++-------- 6 files changed, 43 insertions(+), 20 deletions(-) create mode 100644 changelog.d/18063.misc diff --git a/changelog.d/18063.misc b/changelog.d/18063.misc new file mode 100644 index 00000000000..424bf25408f --- /dev/null +++ b/changelog.d/18063.misc @@ -0,0 +1 @@ +Refactor `get_profile` to no longer include fields with a value of `None`. diff --git a/synapse/api/constants.py b/synapse/api/constants.py index 1206d1e00f3..9806e2b0fe3 100644 --- a/synapse/api/constants.py +++ b/synapse/api/constants.py @@ -320,3 +320,8 @@ class ApprovalNoticeMedium: class Direction(enum.Enum): BACKWARDS = "b" FORWARDS = "f" + + +class ProfileFields: + DISPLAYNAME: Final = "displayname" + AVATAR_URL: Final = "avatar_url" diff --git a/synapse/handlers/profile.py b/synapse/handlers/profile.py index ac4544ca4c0..97e178cbd5c 100644 --- a/synapse/handlers/profile.py +++ b/synapse/handlers/profile.py @@ -22,6 +22,7 @@ import random from typing import TYPE_CHECKING, List, Optional, Union +from synapse.api.constants import ProfileFields from synapse.api.errors import ( AuthError, Codes, @@ -83,7 +84,7 @@ async def get_profile(self, user_id: str, ignore_backoff: bool = True) -> JsonDi Returns: A JSON dictionary. For local queries this will include the displayname and avatar_url - fields. For remote queries it may contain arbitrary information. + fields, if set. For remote queries it may contain arbitrary information. """ target_user = UserID.from_string(user_id) @@ -92,10 +93,13 @@ async def get_profile(self, user_id: str, ignore_backoff: bool = True) -> JsonDi if profileinfo.display_name is None and profileinfo.avatar_url is None: raise SynapseError(404, "Profile was not found", Codes.NOT_FOUND) - return { - "displayname": profileinfo.display_name, - "avatar_url": profileinfo.avatar_url, - } + # Do not include display name or avatar are denoted if unset. + ret = {} + if profileinfo.display_name is not None: + ret[ProfileFields.DISPLAYNAME] = profileinfo.display_name + if profileinfo.avatar_url is not None: + ret[ProfileFields.AVATAR_URL] = profileinfo.avatar_url + return ret else: try: result = await self.federation.make_query( diff --git a/synapse/handlers/sso.py b/synapse/handlers/sso.py index ee74289b6c4..cee2eefbb37 100644 --- a/synapse/handlers/sso.py +++ b/synapse/handlers/sso.py @@ -43,7 +43,7 @@ from twisted.web.iweb import IRequest from twisted.web.server import Request -from synapse.api.constants import LoginType +from synapse.api.constants import LoginType, ProfileFields from synapse.api.errors import Codes, NotFoundError, RedirectException, SynapseError from synapse.config.sso import SsoAttributeRequirement from synapse.handlers.device import DeviceHandler @@ -813,9 +813,10 @@ def is_allowed_mime_type(content_type: str) -> bool: # bail if user already has the same avatar profile = await self._profile_handler.get_profile(user_id) - if profile["avatar_url"] is not None: - server_name = profile["avatar_url"].split("/")[-2] - media_id = profile["avatar_url"].split("/")[-1] + if ProfileFields.AVATAR_URL in profile: + avatar_url_parts = profile[ProfileFields.AVATAR_URL].split("/") + server_name = avatar_url_parts[-2] + media_id = avatar_url_parts[-1] if self._is_mine_server_name(server_name): media = await self._media_repo.store.get_local_media(media_id) # type: ignore[has-type] if media is not None and upload_name == media.upload_name: diff --git a/synapse/handlers/user_directory.py b/synapse/handlers/user_directory.py index 1281929d384..f88d39b38f0 100644 --- a/synapse/handlers/user_directory.py +++ b/synapse/handlers/user_directory.py @@ -26,7 +26,13 @@ from twisted.internet.interfaces import IDelayedCall import synapse.metrics -from synapse.api.constants import EventTypes, HistoryVisibility, JoinRules, Membership +from synapse.api.constants import ( + EventTypes, + HistoryVisibility, + JoinRules, + Membership, + ProfileFields, +) from synapse.api.errors import Codes, SynapseError from synapse.handlers.state_deltas import MatchChange, StateDeltasHandler from synapse.metrics.background_process_metrics import run_as_background_process @@ -756,6 +762,10 @@ async def _unsafe_refresh_remote_profiles_for_remote_server( await self.store.update_profile_in_user_dir( user_id, - display_name=non_null_str_or_none(profile.get("displayname")), - avatar_url=non_null_str_or_none(profile.get("avatar_url")), + display_name=non_null_str_or_none( + profile.get(ProfileFields.DISPLAYNAME) + ), + avatar_url=non_null_str_or_none( + profile.get(ProfileFields.AVATAR_URL) + ), ) diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index f6bfd93d3ce..2a2f821427d 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -45,6 +45,7 @@ from twisted.web.resource import Resource from synapse.api import errors +from synapse.api.constants import ProfileFields from synapse.api.errors import SynapseError from synapse.api.presence import UserPresenceState from synapse.config import ConfigError @@ -1086,7 +1087,10 @@ async def update_room_membership( content = {} # Set the profile if not already done by the module. - if "avatar_url" not in content or "displayname" not in content: + if ( + ProfileFields.AVATAR_URL not in content + or ProfileFields.DISPLAYNAME not in content + ): try: # Try to fetch the user's profile. profile = await self._hs.get_profile_handler().get_profile( @@ -1095,8 +1099,8 @@ async def update_room_membership( except SynapseError as e: # If the profile couldn't be found, use default values. profile = { - "displayname": target_user_id.localpart, - "avatar_url": None, + ProfileFields.DISPLAYNAME: target_user_id.localpart, + ProfileFields.AVATAR_URL: None, } if e.code != 404: @@ -1109,11 +1113,9 @@ async def update_room_membership( ) # Set the profile where it needs to be set. - if "avatar_url" not in content: - content["avatar_url"] = profile["avatar_url"] - - if "displayname" not in content: - content["displayname"] = profile["displayname"] + for field_name in [ProfileFields.AVATAR_URL, ProfileFields.DISPLAYNAME]: + if field_name not in content and field_name in profile: + content[field_name] = profile[field_name] event_id, _ = await self._hs.get_room_member_handler().update_membership( requester=requester, From 751effa9358ddc9660a1b1223496a25d9e4c0b2f Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 3 Jan 2025 11:20:01 -0500 Subject: [PATCH 2/3] Use the get_profile method in the servlet. --- synapse/rest/client/profile.py | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/synapse/rest/client/profile.py b/synapse/rest/client/profile.py index 7a95b9445d1..ef59582865f 100644 --- a/synapse/rest/client/profile.py +++ b/synapse/rest/client/profile.py @@ -227,14 +227,7 @@ async def on_GET( user = UserID.from_string(user_id) await self.profile_handler.check_profile_query_allowed(user, requester_user) - displayname = await self.profile_handler.get_displayname(user) - avatar_url = await self.profile_handler.get_avatar_url(user) - - ret = {} - if displayname is not None: - ret["displayname"] = displayname - if avatar_url is not None: - ret["avatar_url"] = avatar_url + ret = await self.profile_handler.get_profile(user_id) return 200, ret From 2984e09d83f418bf6b28df484d3b847f743fd6e4 Mon Sep 17 00:00:00 2001 From: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> Date: Fri, 3 Jan 2025 17:06:46 +0000 Subject: [PATCH 3/3] fix typo in comment --- synapse/handlers/profile.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/handlers/profile.py b/synapse/handlers/profile.py index 97e178cbd5c..22eedcb54f6 100644 --- a/synapse/handlers/profile.py +++ b/synapse/handlers/profile.py @@ -93,7 +93,7 @@ async def get_profile(self, user_id: str, ignore_backoff: bool = True) -> JsonDi if profileinfo.display_name is None and profileinfo.avatar_url is None: raise SynapseError(404, "Profile was not found", Codes.NOT_FOUND) - # Do not include display name or avatar are denoted if unset. + # Do not include display name or avatar if unset. ret = {} if profileinfo.display_name is not None: ret[ProfileFields.DISPLAYNAME] = profileinfo.display_name