On upgrade room only send canonical alias once. (#7547)

Instead of doing a complicated dance of deleting and moving aliases one
by one, which sends a canonical alias update into the old room for each
one, lets do it all in one go.

This also changes the function to move *all* local alias events to the new
room, however that happens later on anyway.

changelog.d/7547.misc

@@ -0,0 +1 @@
+On upgrade room only send canonical alias once. 

synapse/handlers/room.py

@@ -439,73 +439,78 @@ class RoomCreationHandler(BaseHandler):
         new_room_id: str,
         old_room_state: StateMap[str],
     ):
-        directory_handler = self.hs.get_handlers().directory_handler
-
-        aliases = await self.store.get_aliases_for_room(old_room_id)
-
         # check to see if we have a canonical alias.
         canonical_alias_event = None
         canonical_alias_event_id = old_room_state.get((EventTypes.CanonicalAlias, ""))
         if canonical_alias_event_id:
             canonical_alias_event = await self.store.get_event(canonical_alias_event_id)
 
-        # first we try to remove the aliases from the old room (we suppress sending
+        await self.store.update_aliases_for_room(old_room_id, new_room_id)
-        # the room_aliases event until the end).
-        #
-        # Note that we'll only be able to remove aliases that (a) aren't owned by an AS,
-        # and (b) unless the user is a server admin, which the user created.
-        #
-        # This is probably correct - given we don't allow such aliases to be deleted
-        # normally, it would be odd to allow it in the case of doing a room upgrade -
-        # but it makes the upgrade less effective, and you have to wonder why a room
-        # admin can't remove aliases that point to that room anyway.
-        # (cf https://github.com/matrix-org/synapse/issues/2360)
-        #
-        removed_aliases = []
-        for alias_str in aliases:
-            alias = RoomAlias.from_string(alias_str)
-            try:
-                await directory_handler.delete_association(requester, alias)
-                removed_aliases.append(alias_str)
-            except SynapseError as e:
-                logger.warning("Unable to remove alias %s from old room: %s", alias, e)
 
-        # if we didn't find any aliases, or couldn't remove anyway, we can skip the rest
+        if not canonical_alias_event:
-        # of this.
-        if not removed_aliases:
             return
 
-        # we can now add any aliases we successfully removed to the new room.
+        # If there is a canonical alias we need to update the one in the old
-        for alias in removed_aliases:
+        # room and set one in the new one.
-            try:
+        old_canonical_alias_content = dict(canonical_alias_event.content)
-                await directory_handler.create_association(
+        new_canonical_alias_content = {}
-                    requester,
+
-                    RoomAlias.from_string(alias),
+        canonical = canonical_alias_event.content.get("alias")
-                    new_room_id,
+        if canonical and self.hs.is_mine_id(canonical):
-                    servers=(self.hs.hostname,),
+            new_canonical_alias_content["alias"] = canonical
-                    check_membership=False,
+            old_canonical_alias_content.pop("alias", None)
-                )
+
-                logger.info("Moved alias %s to new room", alias)
+        # We convert to a list as it will be a Tuple.
-            except SynapseError as e:
+        old_alt_aliases = list(old_canonical_alias_content.get("alt_aliases", []))
-                # I'm not really expecting this to happen, but it could if the spam
+        if old_alt_aliases:
-                # checking module decides it shouldn't, or similar.
+            old_canonical_alias_content["alt_aliases"] = old_alt_aliases
-                logger.error("Error adding alias %s to new room: %s", alias, e)
+            new_alt_aliases = new_canonical_alias_content.setdefault("alt_aliases", [])
+            for alias in canonical_alias_event.content.get("alt_aliases", []):
+                try:
+                    if self.hs.is_mine_id(alias):
+                        new_alt_aliases.append(alias)
+                        old_alt_aliases.remove(alias)
+                except Exception:
+                    logger.info(
+                        "Invalid alias %s in canonical alias event %s",
+                        alias,
+                        canonical_alias_event_id,
+                    )
+
+            if not old_alt_aliases:
+                old_canonical_alias_content.pop("alt_aliases")
 
         # If a canonical alias event existed for the old room, fire a canonical
         # alias event for the new room with a copy of the information.
         try:
-            if canonical_alias_event:
+            await self.event_creation_handler.create_and_send_nonmember_event(
-                await self.event_creation_handler.create_and_send_nonmember_event(
+                requester,
-                    requester,
+                {
-                    {
+                    "type": EventTypes.CanonicalAlias,
-                        "type": EventTypes.CanonicalAlias,
+                    "state_key": "",
-                        "state_key": "",
+                    "room_id": old_room_id,
-                        "room_id": new_room_id,
+                    "sender": requester.user.to_string(),
-                        "sender": requester.user.to_string(),
+                    "content": old_canonical_alias_content,
-                        "content": canonical_alias_event.content,
+                },
-                    },
+                ratelimit=False,
-                    ratelimit=False,
+            )
-                )
+        except SynapseError as e:
+            # again I'm not really expecting this to fail, but if it does, I'd rather
+            # we returned the new room to the client at this point.
+            logger.error("Unable to send updated alias events in old room: %s", e)
+
+        try:
+            await self.event_creation_handler.create_and_send_nonmember_event(
+                requester,
+                {
+                    "type": EventTypes.CanonicalAlias,
+                    "state_key": "",
+                    "room_id": new_room_id,
+                    "sender": requester.user.to_string(),
+                    "content": new_canonical_alias_content,
+                },
+                ratelimit=False,
+            )
         except SynapseError as e:
             # again I'm not really expecting this to fail, but if it does, I'd rather
             # we returned the new room to the client at this point.