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