From e5910059fe1e788acd847b91dddc230506e4701a Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 7 Sep 2021 14:20:11 -0400 Subject: [PATCH 1/3] Handle upgrading spaces. --- synapse/handlers/room.py | 14 ++++++- tests/rest/client/test_upgrade_room.py | 55 ++++++++++++++++++++++++-- 2 files changed, 63 insertions(+), 6 deletions(-) diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index 0235fd09b4da..42ff5805620a 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -33,6 +33,7 @@ Membership, RoomCreationPreset, RoomEncryptionAlgorithms, + RoomTypes, ) from synapse.api.errors import ( AuthError, @@ -397,7 +398,7 @@ async def clone_existing_room( initial_state = {} # Replicate relevant room events - types_to_copy = ( + types_to_copy: List[Tuple[str, Optional[str]]] = [ (EventTypes.JoinRules, ""), (EventTypes.Name, ""), (EventTypes.Topic, ""), @@ -408,7 +409,16 @@ async def clone_existing_room( (EventTypes.ServerACL, ""), (EventTypes.RelatedGroups, ""), (EventTypes.PowerLevels, ""), - ) + ] + + # If the old room was a space, copy over the room type and the rooms in + # the space. + if ( + old_room_create_event.content.get(EventContentFields.ROOM_TYPE) + == RoomTypes.SPACE + ): + creation_content[EventContentFields.ROOM_TYPE] = RoomTypes.SPACE + types_to_copy.append((EventTypes.SpaceChild, None)) old_room_state_ids = await self.store.get_filtered_current_state_ids( old_room_id, StateFilter.from_types(types_to_copy) diff --git a/tests/rest/client/test_upgrade_room.py b/tests/rest/client/test_upgrade_room.py index 72f976d8e2ed..a4a4d2424c20 100644 --- a/tests/rest/client/test_upgrade_room.py +++ b/tests/rest/client/test_upgrade_room.py @@ -13,9 +13,11 @@ # limitations under the License. from typing import Optional +from synapse.api.constants import EventContentFields, EventTypes, RoomTypes from synapse.config.server import DEFAULT_ROOM_VERSION from synapse.rest import admin from synapse.rest.client import login, room, room_upgrade_rest_servlet +from synapse.server import HomeServer from tests import unittest from tests.server import FakeChannel @@ -29,9 +31,8 @@ class UpgradeRoomTest(unittest.HomeserverTestCase): room_upgrade_rest_servlet.register_servlets, ] - def prepare(self, reactor, clock, hs): + def prepare(self, reactor, clock, hs: "HomeServer"): self.store = hs.get_datastore() - self.handler = hs.get_user_directory_handler() self.creator = self.register_user("creator", "pass") self.creator_token = self.login(self.creator, "pass") @@ -42,13 +43,18 @@ def prepare(self, reactor, clock, hs): self.room_id = self.helper.create_room_as(self.creator, tok=self.creator_token) self.helper.join(self.room_id, self.other, tok=self.other_token) - def _upgrade_room(self, token: Optional[str] = None) -> FakeChannel: + def _upgrade_room( + self, token: Optional[str] = None, room_id: Optional[str] = None + ) -> FakeChannel: # We never want a cached response. self.reactor.advance(5 * 60 + 1) + if room_id is None: + room_id = self.room_id + return self.make_request( "POST", - "/_matrix/client/r0/rooms/%s/upgrade" % self.room_id, + f"/_matrix/client/r0/rooms/{room_id}/upgrade", # This will upgrade a room to the same version, but that's fine. content={"new_version": DEFAULT_ROOM_VERSION}, access_token=token or self.creator_token, @@ -157,3 +163,44 @@ def test_power_levels_tombstone(self): tok=self.creator_token, ) self.assertNotIn(self.other, power_levels["users"]) + + def test_space(self): + """Test upgrading a space.""" + + # Create a space. + space_id = self.helper.create_room_as( + self.creator, + tok=self.creator_token, + extra_content={ + "creation_content": {EventContentFields.ROOM_TYPE: RoomTypes.SPACE} + }, + ) + + # Add the room as a child room. + self.helper.send_state( + space_id, + event_type=EventTypes.SpaceChild, + body={"via": [self.hs.hostname]}, + tok=self.creator_token, + state_key=self.room_id, + ) + + # Upgrade the room! + channel = self._upgrade_room(room_id=space_id) + self.assertEquals(200, channel.code, channel.result) + self.assertIn("replacement_room", channel.json_body) + + new_space_id = channel.json_body["replacement_room"] + + state_ids = self.get_success(self.store.get_current_state_ids(new_space_id)) + + # Ensure the new room is still a space. + create_event = self.get_success( + self.store.get_event(state_ids[(EventTypes.Create, "")]) + ) + self.assertEqual( + create_event.content.get(EventContentFields.ROOM_TYPE), RoomTypes.SPACE + ) + + # The child link should have been copied over. + self.assertIn((EventTypes.SpaceChild, self.room_id), state_ids) From 1903b0639bb405d99b07f0617a583ae9abb044e7 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 7 Sep 2021 14:26:54 -0400 Subject: [PATCH 2/3] Newsfragment --- changelog.d/10774.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/10774.bugfix diff --git a/changelog.d/10774.bugfix b/changelog.d/10774.bugfix new file mode 100644 index 000000000000..5c2f6f8ade12 --- /dev/null +++ b/changelog.d/10774.bugfix @@ -0,0 +1 @@ +Properly handle room upgrades of spaces. From 802578dbb8422535ff8de9ea65c2f9555831bb77 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 9 Sep 2021 08:29:33 -0400 Subject: [PATCH 3/3] Ignore space child events with empty content when upgrading. --- synapse/handlers/room.py | 5 +++++ tests/rest/client/test_upgrade_room.py | 12 ++++++++++++ 2 files changed, 17 insertions(+) diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index 42ff5805620a..bc2c1e1e7bab 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -429,6 +429,11 @@ async def clone_existing_room( for k, old_event_id in old_room_state_ids.items(): old_event = old_room_state_events.get(old_event_id) if old_event: + # If the event is an space child event with empty content, it was + # removed from the space and should be ignored. + if k[0] == EventTypes.SpaceChild and not old_event.content: + continue + initial_state[k] = old_event.content # deep-copy the power-levels event before we start modifying it diff --git a/tests/rest/client/test_upgrade_room.py b/tests/rest/client/test_upgrade_room.py index a4a4d2424c20..a42388b26ffb 100644 --- a/tests/rest/client/test_upgrade_room.py +++ b/tests/rest/client/test_upgrade_room.py @@ -185,6 +185,16 @@ def test_space(self): state_key=self.room_id, ) + # Also add a room that was removed. + old_room_id = "!notaroom:" + self.hs.hostname + self.helper.send_state( + space_id, + event_type=EventTypes.SpaceChild, + body={}, + tok=self.creator_token, + state_key=old_room_id, + ) + # Upgrade the room! channel = self._upgrade_room(room_id=space_id) self.assertEquals(200, channel.code, channel.result) @@ -204,3 +214,5 @@ def test_space(self): # The child link should have been copied over. self.assertIn((EventTypes.SpaceChild, self.room_id), state_ids) + # The child that was removed should not be copied over. + self.assertNotIn((EventTypes.SpaceChild, old_room_id), state_ids)