Refactor get_profile: do not return missing fields. (#18063)

Refactor `get_profile` to avoid returning "empty" (`None` / `null`)
fields. Currently this is not very important, but will be more useful
once #17488 lands. It does update the servlet to use this now which has
a minor change in behavior: additional fields served over federation
will now be properly sent back to clients.

It also adds constants for `avatar_url` / `displayname` although I did
not attempt to use it everywhere possible.

changelog.d/18063.misc

@@ -0,0 +1 @@
+Refactor `get_profile` to no longer include fields with a value of `None`.

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"

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 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(

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:

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)
+                    ),
                 )

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,

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