Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Use RoomVersion objects #10934

Merged
merged 5 commits into from
Sep 29, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/10934.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Refactor various parts of the codebase to use `RoomVersion` objects instead of room version identifier strings.
20 changes: 0 additions & 20 deletions synapse/events/builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand Down Expand Up @@ -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(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should rename this to new(...) now?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kinda like having the verbose name. It makes grepping more effective, for a start.

self, room_version: RoomVersion, key_values: dict
) -> EventBuilder:
Expand Down
46 changes: 26 additions & 20 deletions synapse/handlers/federation.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we likely could have kept the variable names as room_version?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah we could; however renaming them made it easier for me to make sure I was updating all the places they were referenced, and I think it does no harm to have renamed them.

builder = self.event_builder_factory.for_room_version(
room_version_obj,
{
"type": EventTypes.Member,
"content": {"membership": Membership.LEAVE},
Expand All @@ -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)
Expand Down Expand Up @@ -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},
Expand All @@ -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)
Expand Down Expand Up @@ -1245,16 +1245,18 @@ 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(
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
)

EventValidator().validate_new(event, self.config)
Expand All @@ -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)
Expand Down Expand Up @@ -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, 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)
Expand All @@ -1331,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,
Expand Down Expand Up @@ -1363,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
Expand Down
27 changes: 21 additions & 6 deletions synapse/handlers/message.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
NotFoundError,
ShadowBanError,
SynapseError,
UnsupportedRoomVersionError,
)
from synapse.api.room_versions import KNOWN_ROOM_VERSIONS, RoomVersions
from synapse.api.urls import ConsentURIBuilder
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions synapse/handlers/room.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down