summary refs log tree commit diff
diff options
context:
space:
mode:
-rw-r--r--changelog.d/17164.bugfix1
-rw-r--r--synapse/handlers/message.py32
-rw-r--r--synapse/handlers/room_member.py35
-rw-r--r--tests/handlers/test_room_member.py21
4 files changed, 54 insertions, 35 deletions
diff --git a/changelog.d/17164.bugfix b/changelog.d/17164.bugfix
new file mode 100644
index 0000000000..597e2f14b0
--- /dev/null
+++ b/changelog.d/17164.bugfix
@@ -0,0 +1 @@
+Fix deduplicating of membership events to not create unused state groups.
diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py
index ccaa5508ff..de5bd44a5f 100644
--- a/synapse/handlers/message.py
+++ b/synapse/handlers/message.py
@@ -496,13 +496,6 @@ class EventCreationHandler:
 
         self.room_prejoin_state_types = self.hs.config.api.room_prejoin_state
 
-        self.membership_types_to_include_profile_data_in = {
-            Membership.JOIN,
-            Membership.KNOCK,
-        }
-        if self.hs.config.server.include_profile_data_on_invite:
-            self.membership_types_to_include_profile_data_in.add(Membership.INVITE)
-
         self.send_event = ReplicationSendEventRestServlet.make_client(hs)
         self.send_events = ReplicationSendEventsRestServlet.make_client(hs)
 
@@ -594,8 +587,6 @@ class EventCreationHandler:
         Creates an FrozenEvent object, filling out auth_events, prev_events,
         etc.
 
-        Adds display names to Join membership events.
-
         Args:
             requester
             event_dict: An entire event
@@ -672,29 +663,6 @@ class EventCreationHandler:
 
         self.validator.validate_builder(builder)
 
-        if builder.type == EventTypes.Member:
-            membership = builder.content.get("membership", None)
-            target = UserID.from_string(builder.state_key)
-
-            if membership in self.membership_types_to_include_profile_data_in:
-                # If event doesn't include a display name, add one.
-                profile = self.profile_handler
-                content = builder.content
-
-                try:
-                    if "displayname" not in content:
-                        displayname = await profile.get_displayname(target)
-                        if displayname is not None:
-                            content["displayname"] = displayname
-                    if "avatar_url" not in content:
-                        avatar_url = await profile.get_avatar_url(target)
-                        if avatar_url is not None:
-                            content["avatar_url"] = avatar_url
-                except Exception as e:
-                    logger.info(
-                        "Failed to get profile information for %r: %s", target, e
-                    )
-
         is_exempt = await self._is_exempt_from_privacy_policy(builder, requester)
         if require_consent and not is_exempt:
             await self.assert_accepted_privacy_policy(requester)
diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py
index 655c78e150..51b9772329 100644
--- a/synapse/handlers/room_member.py
+++ b/synapse/handlers/room_member.py
@@ -106,6 +106,13 @@ class RoomMemberHandler(metaclass=abc.ABCMeta):
         self.event_auth_handler = hs.get_event_auth_handler()
         self._worker_lock_handler = hs.get_worker_locks_handler()
 
+        self._membership_types_to_include_profile_data_in = {
+            Membership.JOIN,
+            Membership.KNOCK,
+        }
+        if self.hs.config.server.include_profile_data_on_invite:
+            self._membership_types_to_include_profile_data_in.add(Membership.INVITE)
+
         self.member_linearizer: Linearizer = Linearizer(name="member")
         self.member_as_limiter = Linearizer(max_count=10, name="member_as_limiter")
 
@@ -785,9 +792,8 @@ class RoomMemberHandler(metaclass=abc.ABCMeta):
         if (
             not self.allow_per_room_profiles and not is_requester_server_notices_user
         ) or requester.shadow_banned:
-            # Strip profile data, knowing that new profile data will be added to the
-            # event's content in event_creation_handler.create_event() using the target's
-            # global profile.
+            # Strip profile data, knowing that new profile data will be added to
+            # the event's content below using the target's global profile.
             content.pop("displayname", None)
             content.pop("avatar_url", None)
 
@@ -823,6 +829,29 @@ class RoomMemberHandler(metaclass=abc.ABCMeta):
         if action in ["kick", "unban"]:
             effective_membership_state = "leave"
 
+        if effective_membership_state not in Membership.LIST:
+            raise SynapseError(400, "Invalid membership key")
+
+        # Add profile data for joins etc, if no per-room profile.
+        if (
+            effective_membership_state
+            in self._membership_types_to_include_profile_data_in
+        ):
+            # If event doesn't include a display name, add one.
+            profile = self.profile_handler
+
+            try:
+                if "displayname" not in content:
+                    displayname = await profile.get_displayname(target)
+                    if displayname is not None:
+                        content["displayname"] = displayname
+                if "avatar_url" not in content:
+                    avatar_url = await profile.get_avatar_url(target)
+                    if avatar_url is not None:
+                        content["avatar_url"] = avatar_url
+            except Exception as e:
+                logger.info("Failed to get profile information for %r: %s", target, e)
+
         # if this is a join with a 3pid signature, we may need to turn a 3pid
         # invite into a normal invite before we can handle the join.
         if third_party_signed is not None:
diff --git a/tests/handlers/test_room_member.py b/tests/handlers/test_room_member.py
index df43ce581c..213a66ed1a 100644
--- a/tests/handlers/test_room_member.py
+++ b/tests/handlers/test_room_member.py
@@ -407,3 +407,24 @@ class RoomMemberMasterHandlerTestCase(HomeserverTestCase):
         self.assertFalse(
             self.get_success(self.store.did_forget(self.alice, self.room_id))
         )
+
+    def test_deduplicate_joins(self) -> None:
+        """
+        Test that calling /join multiple times does not store a new state group.
+        """
+
+        self.helper.join(self.room_id, user=self.bob, tok=self.bob_token)
+
+        sql = "SELECT COUNT(*) FROM state_groups WHERE room_id = ?"
+        rows = self.get_success(
+            self.store.db_pool.execute("test_deduplicate_joins", sql, self.room_id)
+        )
+        initial_count = rows[0][0]
+
+        self.helper.join(self.room_id, user=self.bob, tok=self.bob_token)
+        rows = self.get_success(
+            self.store.db_pool.execute("test_deduplicate_joins", sql, self.room_id)
+        )
+        new_count = rows[0][0]
+
+        self.assertEqual(initial_count, new_count)