Use `RoomVersion` objects (#10934)

Various refactors to use `RoomVersion` objects instead of room version identifiers.

5 files changed, 50 insertions, 48 deletions

diff --git a/changelog.d/10934.misc b/changelog.d/10934.misc
new file mode 100644
index 0000000000..56c640ec9e
--- /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.
diff --git a/synapse/events/builder.py b/synapse/events/builder.py
index 87e2bb123b..50f2a4c1f4 100644
--- a/synapse/events/builder.py
+++ b/synapse/events/builder.py
@@ -18,10 +18,8 @@ import attr
 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 @@ class EventBuilderFactory:
         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 b17ef2a9a1..16c435ee86 100644
--- a/synapse/handlers/federation.py
+++ b/synapse/handlers/federation.py
@@ -718,8 +718,8 @@ class FederationHandler(BaseHandler):
                         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,
@@ -897,9 +897,9 @@ class FederationHandler(BaseHandler):
             )
             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 @@ class FederationHandler(BaseHandler):
             # 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 @@ class FederationHandler(BaseHandler):
             )
             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 @@ class FederationHandler(BaseHandler):
             # 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 @@ class FederationHandler(BaseHandler):
         }
 
         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 @@ class FederationHandler(BaseHandler):
             )
 
             event, context = await self.add_display_name_to_third_party_invite(
-                room_version, event_dict, event, context
+                room_version_obj, event_dict, event, context
             )
 
             EventValidator().validate_new(event, self.config)
@@ -1265,7 +1267,7 @@ class FederationHandler(BaseHandler):
 
             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 @@ class FederationHandler(BaseHandler):
 
         """
         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, 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)
@@ -1331,7 +1335,7 @@ class FederationHandler(BaseHandler):
 
     async def add_display_name_to_third_party_invite(
         self,
-        room_version: str,
+        room_version_obj: RoomVersion,
         event_dict: JsonDict,
         event: EventBase,
         context: EventContext,
@@ -1363,7 +1367,9 @@ class FederationHandler(BaseHandler):
             # 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
diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py
index 07aadf3f3c..39c18ecf99 100644
--- a/synapse/handlers/message.py
+++ b/synapse/handlers/message.py
@@ -40,6 +40,7 @@ from synapse.api.errors import (
     NotFoundError,
     ShadowBanError,
     SynapseError,
+    UnsupportedRoomVersionError,
 )
 from synapse.api.room_versions import KNOWN_ROOM_VERSIONS, RoomVersions
 from synapse.api.urls import ConsentURIBuilder
@@ -550,16 +551,22 @@ class EventCreationHandler:
         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)
 
@@ -1070,9 +1077,17 @@ class EventCreationHandler:
             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
@@ -1082,7 +1097,7 @@ class EventCreationHandler:
         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)
diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py
index 8fede5e935..dc4fab2223 100644
--- a/synapse/handlers/room.py
+++ b/synapse/handlers/room.py
@@ -237,9 +237,9 @@ class RoomCreationHandler(BaseHandler):
                 },
             },
         )
-        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(