From 6b7a65d944734ad22927c5e0e528145977139c02 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 28 Sep 2021 14:41:34 +0100 Subject: [PATCH 1/5] Use `RoomVersion` objects in more places Some fairly mechanical changes to replace calls to `get_room_version_id` with `get_room_version`, and `event_builder_factory.new` with `event_builder_factory.for_room_version`. --- synapse/handlers/federation.py | 36 +++++++++++++++++++--------------- synapse/handlers/room.py | 4 ++-- 2 files changed, 22 insertions(+), 18 deletions(-) diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index b17ef2a9a104..4b669238b574 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -897,9 +897,9 @@ async def on_make_leave_request( ) raise SynapseError(403, "User not from origin", Codes.FORBIDDEN) - room_version = await self.store.get_room_version_id(room_id) - builder = self.event_builder_factory.new( - room_version, + room_version_obj = await self.store.get_room_version(room_id) + builder = self.event_builder_factory.for_room_version( + room_version_obj, { "type": EventTypes.Member, "content": {"membership": Membership.LEAVE}, @@ -917,7 +917,7 @@ async def on_make_leave_request( # The remote hasn't signed it yet, obviously. We'll do the full checks # when we get the event back in `on_send_leave_request` await self._event_auth_handler.check_from_context( - room_version, event, context, do_sig_check=False + room_version_obj.identifier, event, context, do_sig_check=False ) except AuthError as e: logger.warning("Failed to create new leave %r because %s", event, e) @@ -949,10 +949,10 @@ async def on_make_knock_request( ) raise SynapseError(403, "User not from origin", Codes.FORBIDDEN) - room_version = await self.store.get_room_version_id(room_id) + room_version_obj = await self.store.get_room_version(room_id) - builder = self.event_builder_factory.new( - room_version, + builder = self.event_builder_factory.for_room_version( + room_version_obj, { "type": EventTypes.Member, "content": {"membership": Membership.KNOCK}, @@ -979,7 +979,7 @@ async def on_make_knock_request( # The remote hasn't signed it yet, obviously. We'll do the full checks # when we get the event back in `on_send_knock_request` await self._event_auth_handler.check_from_context( - room_version, event, context, do_sig_check=False + room_version_obj.identifier, event, context, do_sig_check=False ) except AuthError as e: logger.warning("Failed to create new knock %r because %s", event, e) @@ -1245,8 +1245,10 @@ async def exchange_third_party_invite( } if await self._event_auth_handler.check_host_in_room(room_id, self.hs.hostname): - room_version = await self.store.get_room_version_id(room_id) - builder = self.event_builder_factory.new(room_version, event_dict) + room_version_obj = await self.store.get_room_version(room_id) + builder = self.event_builder_factory.for_room_version( + room_version_obj, event_dict + ) EventValidator().validate_builder(builder) event, context = await self.event_creation_handler.create_new_client_event( @@ -1254,7 +1256,7 @@ async def exchange_third_party_invite( ) event, context = await self.add_display_name_to_third_party_invite( - room_version, event_dict, event, context + room_version_obj.identifier, event_dict, event, context ) EventValidator().validate_new(event, self.config) @@ -1265,7 +1267,7 @@ async def exchange_third_party_invite( try: await self._event_auth_handler.check_from_context( - room_version, event, context + room_version_obj.identifier, event, context ) except AuthError as e: logger.warning("Denying new third party invite %r because %s", event, e) @@ -1299,22 +1301,24 @@ async def on_exchange_third_party_invite_request( """ assert_params_in_dict(event_dict, ["room_id"]) - room_version = await self.store.get_room_version_id(event_dict["room_id"]) + room_version_obj = await self.store.get_room_version(event_dict["room_id"]) # NB: event_dict has a particular specced format we might need to fudge # if we change event formats too much. - builder = self.event_builder_factory.new(room_version, event_dict) + builder = self.event_builder_factory.for_room_version( + room_version_obj, event_dict + ) event, context = await self.event_creation_handler.create_new_client_event( builder=builder ) event, context = await self.add_display_name_to_third_party_invite( - room_version, event_dict, event, context + room_version_obj.identifier, event_dict, event, context ) try: await self._event_auth_handler.check_from_context( - room_version, event, context + room_version_obj.identifier, event, context ) except AuthError as e: logger.warning("Denying third party invite %r because %s", event, e) diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index 8fede5e935d0..dc4fab22238d 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -237,9 +237,9 @@ async def _upgrade_room( }, }, ) - old_room_version = await self.store.get_room_version_id(old_room_id) + old_room_version = await self.store.get_room_version(old_room_id) await self._event_auth_handler.check_from_context( - old_room_version, tombstone_event, tombstone_context + old_room_version.identifier, tombstone_event, tombstone_context ) await self.clone_existing_room( From 1059c767012146e13645426b34a5cf5294fc4a65 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 28 Sep 2021 14:44:44 +0100 Subject: [PATCH 2/5] `add_display_name_to_third_party_invite`: take a RoomVersion object --- synapse/handlers/federation.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index 4b669238b574..b2d50e794cf9 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -1256,7 +1256,7 @@ async def exchange_third_party_invite( ) event, context = await self.add_display_name_to_third_party_invite( - room_version_obj.identifier, event_dict, event, context + room_version_obj, event_dict, event, context ) EventValidator().validate_new(event, self.config) @@ -1313,7 +1313,7 @@ async def on_exchange_third_party_invite_request( builder=builder ) event, context = await self.add_display_name_to_third_party_invite( - room_version_obj.identifier, event_dict, event, context + room_version_obj, event_dict, event, context ) try: @@ -1335,7 +1335,7 @@ async def on_exchange_third_party_invite_request( async def add_display_name_to_third_party_invite( self, - room_version: str, + room_version_obj: RoomVersion, event_dict: JsonDict, event: EventBase, context: EventContext, @@ -1367,7 +1367,9 @@ async def add_display_name_to_third_party_invite( # auth checks. If we need the invite and don't have it then the # auth check code will explode appropriately. - builder = self.event_builder_factory.new(room_version, event_dict) + builder = self.event_builder_factory.for_room_version( + room_version_obj, event_dict + ) EventValidator().validate_builder(builder) event, context = await self.event_creation_handler.create_new_client_event( builder=builder From 1eb47bfbc8ed15f0ad4a792119d7bbb07b978d28 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 28 Sep 2021 14:46:22 +0100 Subject: [PATCH 3/5] `MessageHandler`: use `RoomVersion` objects use `store.get_room_version` and `event_builder_factory.for_room_version` --- synapse/handlers/message.py | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index c66aefe2c4c5..6dca5e8059c8 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -39,6 +39,7 @@ NotFoundError, ShadowBanError, SynapseError, + UnsupportedRoomVersionError, ) from synapse.api.room_versions import KNOWN_ROOM_VERSIONS, RoomVersions from synapse.api.urls import ConsentURIBuilder @@ -549,16 +550,22 @@ async def create_event( await self.auth.check_auth_blocking(requester=requester) if event_dict["type"] == EventTypes.Create and event_dict["state_key"] == "": - room_version = event_dict["content"]["room_version"] + room_version_id = event_dict["content"]["room_version"] + room_version_obj = KNOWN_ROOM_VERSIONS.get(room_version_id) + if not room_version_obj: + # this can happen if support is withdrawn for a room version + raise UnsupportedRoomVersionError(room_version_id) else: try: - room_version = await self.store.get_room_version_id( + room_version_obj = await self.store.get_room_version( event_dict["room_id"] ) except NotFoundError: raise AuthError(403, "Unknown room") - builder = self.event_builder_factory.new(room_version, event_dict) + builder = self.event_builder_factory.for_room_version( + room_version_obj, event_dict + ) self.validator.validate_builder(builder) @@ -1069,9 +1076,17 @@ async def handle_new_client_event( EventTypes.Create, "", ): - room_version = event.content.get("room_version", RoomVersions.V1.identifier) + room_version_id = event.content.get( + "room_version", RoomVersions.V1.identifier + ) + room_version_obj = KNOWN_ROOM_VERSIONS.get(room_version_id) + if not room_version_obj: + raise UnsupportedRoomVersionError( + "Attempt to create a room with unsupported room version %s" + % (room_version_id,) + ) else: - room_version = await self.store.get_room_version_id(event.room_id) + room_version_obj = await self.store.get_room_version(event.room_id) if event.internal_metadata.is_out_of_band_membership(): # the only sort of out-of-band-membership events we expect to see here are @@ -1081,7 +1096,7 @@ async def handle_new_client_event( else: try: await self._event_auth_handler.check_from_context( - room_version, event, context + room_version_obj.identifier, event, context ) except AuthError as err: logger.warning("Denying new event %r because %s", event, err) From 0ba748a0102209de18c0dc5028db8576b371d8b7 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 28 Sep 2021 15:44:58 +0100 Subject: [PATCH 4/5] Remove `EventBuilderFactory.new` Finally get rid of this deprecated function. --- synapse/events/builder.py | 20 -------------------- synapse/handlers/federation.py | 4 ++-- 2 files changed, 2 insertions(+), 22 deletions(-) diff --git a/synapse/events/builder.py b/synapse/events/builder.py index 87e2bb123b6d..50f2a4c1f418 100644 --- a/synapse/events/builder.py +++ b/synapse/events/builder.py @@ -18,10 +18,8 @@ from nacl.signing import SigningKey from synapse.api.constants import MAX_DEPTH -from synapse.api.errors import UnsupportedRoomVersionError from synapse.api.room_versions import ( KNOWN_EVENT_FORMAT_VERSIONS, - KNOWN_ROOM_VERSIONS, EventFormatVersions, RoomVersion, ) @@ -197,24 +195,6 @@ def __init__(self, hs: "HomeServer"): self.state = hs.get_state_handler() self._event_auth_handler = hs.get_event_auth_handler() - def new(self, room_version: str, key_values: dict) -> EventBuilder: - """Generate an event builder appropriate for the given room version - - Deprecated: use for_room_version with a RoomVersion object instead - - Args: - room_version: Version of the room that we're creating an event builder for - key_values: Fields used as the basis of the new event - - Returns: - EventBuilder - """ - v = KNOWN_ROOM_VERSIONS.get(room_version) - if not v: - # this can happen if support is withdrawn for a room version - raise UnsupportedRoomVersionError() - return self.for_room_version(v, key_values) - def for_room_version( self, room_version: RoomVersion, key_values: dict ) -> EventBuilder: diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index b2d50e794cf9..16c435ee866a 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -718,8 +718,8 @@ async def on_make_join_request( state_ids, ) - builder = self.event_builder_factory.new( - room_version.identifier, + builder = self.event_builder_factory.for_room_version( + room_version, { "type": EventTypes.Member, "content": event_content, From 1dfaf363ed5279ade7f545e31bbc4e40ce0a5122 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 28 Sep 2021 15:56:00 +0100 Subject: [PATCH 5/5] changelog --- changelog.d/10934.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/10934.misc diff --git a/changelog.d/10934.misc b/changelog.d/10934.misc new file mode 100644 index 000000000000..56c640ec9e91 --- /dev/null +++ b/changelog.d/10934.misc @@ -0,0 +1 @@ +Refactor various parts of the codebase to use `RoomVersion` objects instead of room version identifier strings.