Refactor the code that sends state events into new rooms

changelog.d/13291.misc

@@ -0,0 +1 @@
+Refactor the code that calculates the state events to send into new rooms.

synapse/handlers/room.py

@@ -558,7 +558,7 @@ async def clone_existing_room(
             new_room_id,
             # we expect to override all the presets with initial_state, so this is
             # somewhat arbitrary.
-            preset_config=RoomCreationPreset.PRIVATE_CHAT,
+            room_preset_identifier=RoomCreationPreset.PRIVATE_CHAT,
             invite_list=[],
             initial_state=initial_state,
             creation_content=creation_content,
@@ -871,7 +871,7 @@ async def create_room(
                 check_membership=False,
             )
 
-        preset_config = config.get(
+        room_preset_identifier = config.get(
             "preset",
             RoomCreationPreset.PRIVATE_CHAT
             if visibility == "private"
@@ -896,7 +896,7 @@ async def create_room(
         ) = await self._send_events_for_new_room(
             requester,
             room_id,
-            preset_config=preset_config,
+            room_preset_identifier=room_preset_identifier,
             invite_list=invite_list,
             initial_state=initial_state,
             creation_content=creation_content,
@@ -1020,9 +1020,9 @@ async def _send_events_for_new_room(
         self,
         creator: Requester,
         room_id: str,
-        preset_config: str,
+        room_preset_identifier: str,
         invite_list: List[str],
-        initial_state: MutableStateMap,
+        initial_state: MutableStateMap[JsonDict],
         creation_content: JsonDict,
         room_alias: Optional[RoomAlias] = None,
         power_level_content_override: Optional[JsonDict] = None,
@@ -1031,8 +1031,24 @@ async def _send_events_for_new_room(
     ) -> Tuple[int, str, int]:
         """Sends the initial events into a new room.
 
-        `power_level_content_override` doesn't apply when initial state has
-        power level state event content.
+        Args:
+            creator: The creator of the room.
+            room_id: The ID of the room to send events in.
+            invite_list: A list of Matrix user IDs to invite to the room. This is only
+                used by the method to set the power levels of the invitees to 100 if
+                the preset for the room specifies that initial invitees should have ops.
+            initial_state: A map of state key to an event definition or event ID.
+            creation_content: A json dict to use as the value of the "content" field
+                for the room's create event.
+            room_preset_identifier: The identifier of the room preset to use. This
+                determines the events that are sent into the room.
+            room_alias: A room alias to link to the room, if provided.
+            power_level_content_override: A json dictionary that specifies the initial
+                power levels of the room. If `initial_state` contains a 'm.room.power_levels'
+                event then this argument will be ignored.
+            creator_join_profile: The JSON dictionary value of the "content" field for the
+                'm.room.membership' join event that will be sent for the creator of the room.
+            ratelimit: Whether to ratelimit the join event of the room creator.
 
         Returns:
             A tuple containing the stream ID, event ID and depth of the last
@@ -1054,7 +1070,7 @@ def create(etype: str, content: JsonDict, **kwargs: Any) -> JsonDict:
 
             return e
 
-        async def send(etype: str, content: JsonDict, **kwargs: Any) -> int:
+        async def send(etype: str, content: JsonDict, **kwargs: Any) -> None:
             nonlocal last_sent_event_id
             nonlocal depth
 
@@ -1064,7 +1080,7 @@ async def send(etype: str, content: JsonDict, **kwargs: Any) -> int:
             # allow the room creation to complete.
             (
                 sent_event,
-                last_stream_id,
+                last_sent_stream_id,
             ) = await self.event_creation_handler.create_and_send_nonmember_event(
                 creator,
                 event,
@@ -1079,22 +1095,26 @@ async def send(etype: str, content: JsonDict, **kwargs: Any) -> int:
             last_sent_event_id = sent_event.event_id
             depth += 1
 
-            return last_stream_id
-
         try:
-            config = self._presets_dict[preset_config]
+            room_preset_config = self._presets_dict[room_preset_identifier]
         except KeyError:
             raise SynapseError(
-                400, f"'{preset_config}' is not a valid preset", errcode=Codes.BAD_JSON
+                400,
+                f"'{room_preset_identifier}' is not a valid preset",
+                errcode=Codes.BAD_JSON,
             )
 
+        # Create and send the 'm.room.create' event
         creation_content.update({"creator": creator_id})
         await send(etype=EventTypes.Create, content=creation_content)
+        assert last_sent_event_id is not None
 
+        # Create and send the join event for the room creator
         logger.debug("Sending %s in new room", EventTypes.Member)
-        # Room create event must exist at this point
-        assert last_sent_event_id is not None
-        member_event_id, _ = await self.room_member_handler.update_membership(
+        (
+            last_sent_event_id,
+            last_sent_stream_id,
+        ) = await self.room_member_handler.update_membership(
             creator,
             creator.user,
             room_id,
@@ -1105,15 +1125,16 @@ async def send(etype: str, content: JsonDict, **kwargs: Any) -> int:
             prev_event_ids=[last_sent_event_id],
             depth=depth,
         )
-        last_sent_event_id = member_event_id
+
+        # Create a map of (event_type, state_key) -> event json dict.
+        # A dict allows us to easily load up and override state event definitions
+        state_to_send: MutableStateMap[JsonDict] = {}
 
         # We treat the power levels override specially as this needs to be one
         # of the first events that get sent into a room.
         pl_content = initial_state.pop((EventTypes.PowerLevels, ""), None)
         if pl_content is not None:
-            last_sent_stream_id = await send(
-                etype=EventTypes.PowerLevels, content=pl_content
-            )
+            state_to_send[(EventTypes.PowerLevels, "")] = pl_content
         else:
             power_level_content: JsonDict = {
                 "users": {creator_id: 100},
@@ -1137,18 +1158,22 @@ async def send(etype: str, content: JsonDict, **kwargs: Any) -> int:
                 "historical": 100,
             }
 
-            if config["original_invitees_have_ops"]:
+            if room_preset_config["original_invitees_have_ops"]:
                 for invitee in invite_list:
                     power_level_content["users"][invitee] = 100
 
             # If the user supplied a preset name e.g. "private_chat",
             # we apply that preset
-            power_level_content.update(config["power_level_content_override"])
+            power_level_content.update(
+                room_preset_config["power_level_content_override"]
+            )
 
-            # If the server config contains default_power_level_content_override,
+            # If the homeserver config contains default_power_level_content_override,
             # and that contains information for this room preset, apply it.
             if self._default_power_level_content_override:
-                override = self._default_power_level_content_override.get(preset_config)
+                override = self._default_power_level_content_override.get(
+                    room_preset_identifier
+                )
                 if override is not None:
                     power_level_content.update(override)
 
@@ -1157,45 +1182,39 @@ async def send(etype: str, content: JsonDict, **kwargs: Any) -> int:
             if power_level_content_override:
                 power_level_content.update(power_level_content_override)
 
-            last_sent_stream_id = await send(
-                etype=EventTypes.PowerLevels, content=power_level_content
-            )
+            state_to_send[(EventTypes.PowerLevels, "")] = power_level_content
 
-        if room_alias and (EventTypes.CanonicalAlias, "") not in initial_state:
-            last_sent_stream_id = await send(
-                etype=EventTypes.CanonicalAlias,
-                content={"alias": room_alias.to_string()},
-            )
+        if room_alias:
+            state_to_send[(EventTypes.CanonicalAlias, "")] = {
+                "alias": room_alias.to_string()
+            }
 
-        if (EventTypes.JoinRules, "") not in initial_state:
-            last_sent_stream_id = await send(
-                etype=EventTypes.JoinRules, content={"join_rule": config["join_rules"]}
-            )
+        state_to_send[(EventTypes.JoinRules, "")] = {
+            "join_rule": room_preset_config["join_rules"]
+        }
+        state_to_send[(EventTypes.RoomHistoryVisibility, "")] = {
+            "history_visibility": room_preset_config["history_visibility"]
+        }
 
-        if (EventTypes.RoomHistoryVisibility, "") not in initial_state:
-            last_sent_stream_id = await send(
-                etype=EventTypes.RoomHistoryVisibility,
-                content={"history_visibility": config["history_visibility"]},
-            )
+        if room_preset_config["guest_can_join"]:
+            state_to_send[(EventTypes.GuestAccess, "")] = {
+                EventContentFields.GUEST_ACCESS: GuestAccess.CAN_JOIN
+            }
 
-        if config["guest_can_join"]:
-            if (EventTypes.GuestAccess, "") not in initial_state:
-                last_sent_stream_id = await send(
-                    etype=EventTypes.GuestAccess,
-                    content={EventContentFields.GUEST_ACCESS: GuestAccess.CAN_JOIN},
-                )
+        # Override any default state with the user-provided contents of `initial_state`
+        state_to_send.update(initial_state)
 
-        for (etype, state_key), content in initial_state.items():
-            last_sent_stream_id = await send(
-                etype=etype, state_key=state_key, content=content
-            )
+        # Set the encryption state of the room based on the room preset config
+        # FIXME: We should do this before layering initial_state on top, c.f.
+        # https://github.com/matrix-org/synapse/issues/9794.
+        if room_preset_config["encrypted"]:
+            state_to_send[(EventTypes.RoomEncryption, "")] = {
+                "algorithm": RoomEncryptionAlgorithms.DEFAULT
+            }
 
-        if config["encrypted"]:
-            last_sent_stream_id = await send(
-                etype=EventTypes.RoomEncryption,
-                state_key="",
-                content={"algorithm": RoomEncryptionAlgorithms.DEFAULT},
-            )
+        # Send each event in order of its insertion into the dictionary
+        for (event_type, state_key), content in state_to_send.items():
+            await send(etype=event_type, state_key=state_key, content=content)
 
         return last_sent_stream_id, last_sent_event_id, depth