From 155399a145f4a901343e64cf6a98263a922b74a4 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 20 May 2022 11:55:04 +0100 Subject: [PATCH 01/13] Add helper function to get the current state event in the room --- synapse/storage/databases/main/state.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/synapse/storage/databases/main/state.py b/synapse/storage/databases/main/state.py index 18ae8aee295d..c81fadbe9e81 100644 --- a/synapse/storage/databases/main/state.py +++ b/synapse/storage/databases/main/state.py @@ -269,6 +269,23 @@ def _get_filtered_current_state_ids_txn( "get_filtered_current_state_ids", _get_filtered_current_state_ids_txn ) + async def get_current_state_event( + self, room_id: str, event_type: str, state_key: str + ) -> Optional[EventBase]: + """Get the current state event for the given type/state_key.""" + + key = (event_type, state_key) + state_map = await self.get_filtered_current_state_ids( + room_id, StateFilter.from_types((key,)) + ) + event_id = state_map.get(key) + + event = None + if event_id: + event = await self.get_event(event_id, allow_none=True) + + return event + async def get_canonical_alias_for_room(self, room_id: str) -> Optional[str]: """Get canonical alias for room, if any From 94cd2cad4f05e70163d44662dbfa4dc96b679139 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 20 May 2022 11:55:45 +0100 Subject: [PATCH 02/13] Use helper function in auth --- synapse/api/auth.py | 36 +++++++++++++++++------------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/synapse/api/auth.py b/synapse/api/auth.py index 931750668ea2..83a30fe7ad96 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -61,7 +61,6 @@ def __init__(self, hs: "HomeServer"): self.hs = hs self.clock = hs.get_clock() self.store = hs.get_datastores().main - self.state = hs.get_state_handler() self._account_validity_handler = hs.get_account_validity_handler() self.token_cache: LruCache[str, Tuple[str, bool]] = LruCache( @@ -81,7 +80,7 @@ async def check_user_in_room( user_id: str, current_state: Optional[StateMap[EventBase]] = None, allow_departed_users: bool = False, - ) -> EventBase: + ) -> Tuple[str, Optional[str]]: """Check if the user is in the room, or was at some point. Args: room_id: The room to check. @@ -99,29 +98,28 @@ async def check_user_in_room( Raises: AuthError if the user is/was not in the room. Returns: - Membership event for the user if the user was in the - room. This will be the join event if they are currently joined to - the room. This will be the leave event if they have left the room. + The current membership of the user in the room and the + membership event ID of the user. """ - if current_state: - member = current_state.get((EventTypes.Member, user_id), None) - else: - member = await self.state.get_current_state( - room_id=room_id, event_type=EventTypes.Member, state_key=user_id - ) - if member: - membership = member.membership + ( + membership, + member_event_id, + ) = await self.store.get_local_current_membership_for_user_in_room( + user_id=user_id, + room_id=room_id, + ) + if membership: if membership == Membership.JOIN: - return member + return membership, member_event_id # XXX this looks totally bogus. Why do we not allow users who have been banned, # or those who were members previously and have been re-invited? if allow_departed_users and membership == Membership.LEAVE: forgot = await self.store.did_forget(user_id, room_id) if not forgot: - return member + return membership, member_event_id raise AuthError(403, "User %s not in room %s" % (user_id, room_id)) @@ -602,7 +600,8 @@ async def check_can_change_room_list(self, room_id: str, user: UserID) -> bool: # We currently require the user is a "moderator" in the room. We do this # by checking if they would (theoretically) be able to change the # m.room.canonical_alias events - power_level_event = await self.state.get_current_state( + + power_level_event = await self.store.get_current_state_event( room_id, EventTypes.PowerLevels, "" ) @@ -693,12 +692,11 @@ async def check_user_in_room_or_world_readable( # * The user is a non-guest user, and was ever in the room # * The user is a guest user, and has joined the room # else it will throw. - member_event = await self.check_user_in_room( + return await self.check_user_in_room( room_id, user_id, allow_departed_users=allow_departed_users ) - return member_event.membership, member_event.event_id except AuthError: - visibility = await self.state.get_current_state( + visibility = await self.store.get_current_state_event( room_id, EventTypes.RoomHistoryVisibility, "" ) if ( From d882ee6219aa1b2078141b3f4590f889680c7534 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 20 May 2022 11:58:01 +0100 Subject: [PATCH 03/13] Use helper function elsewhere --- synapse/handlers/directory.py | 2 +- synapse/handlers/message.py | 4 +++- synapse/handlers/room_member.py | 2 +- synapse/notifier.py | 2 +- synapse/rest/admin/rooms.py | 7 ++++--- synapse/rest/client/room.py | 2 +- synapse/server_notices/resource_limits_server_notices.py | 4 ++-- tests/handlers/test_directory.py | 2 +- tests/storage/test_purge.py | 3 +-- 9 files changed, 15 insertions(+), 13 deletions(-) diff --git a/synapse/handlers/directory.py b/synapse/handlers/directory.py index 4aa33df884ac..f4730e6dcf21 100644 --- a/synapse/handlers/directory.py +++ b/synapse/handlers/directory.py @@ -319,7 +319,7 @@ async def _update_canonical_alias( Raises: ShadowBanError if the requester has been shadow-banned. """ - alias_event = await self.state.get_current_state( + alias_event = await self.store.get_current_state_event( room_id, EventTypes.CanonicalAlias, "" ) diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index e566ff1f8ed8..f712e8cf7567 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -117,7 +117,9 @@ async def get_room_data( ) if membership == Membership.JOIN: - data = await self.state.get_current_state(room_id, event_type, state_key) + data = await self.store.get_current_state_event( + room_id, event_type, state_key + ) elif membership == Membership.LEAVE: key = (event_type, state_key) # If the membership is not JOIN, then the event ID should exist. diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py index ea876c168de7..8d6b255cf679 100644 --- a/synapse/handlers/room_member.py +++ b/synapse/handlers/room_member.py @@ -1805,7 +1805,7 @@ async def _user_left_room(self, target: UserID, room_id: str) -> None: async def forget(self, user: UserID, room_id: str) -> None: user_id = user.to_string() - member = await self.state_handler.get_current_state( + member = await self.store.get_current_state_event( room_id=room_id, event_type=EventTypes.Member, state_key=user_id ) membership = member.membership if member else None diff --git a/synapse/notifier.py b/synapse/notifier.py index ba23257f5498..4a3bc492e765 100644 --- a/synapse/notifier.py +++ b/synapse/notifier.py @@ -681,7 +681,7 @@ async def _get_room_ids( return joined_room_ids, True async def _is_world_readable(self, room_id: str) -> bool: - state = await self.state_handler.get_current_state( + state = await self.store.get_current_state_event( room_id, EventTypes.RoomHistoryVisibility, "" ) if state and "history_visibility" in state.content: diff --git a/synapse/rest/admin/rooms.py b/synapse/rest/admin/rooms.py index 356d6f74d7ef..b05bda04cbf7 100644 --- a/synapse/rest/admin/rooms.py +++ b/synapse/rest/admin/rooms.py @@ -447,7 +447,7 @@ def __init__(self, hs: "HomeServer"): super().__init__(hs) self.auth = hs.get_auth() self.admin_handler = hs.get_admin_handler() - self.state_handler = hs.get_state_handler() + self.store = hs.get_datastores().main self.is_mine = hs.is_mine async def on_POST( @@ -489,8 +489,9 @@ async def on_POST( ) # send invite if room has "JoinRules.INVITE" - room_state = await self.state_handler.get_current_state(room_id) - join_rules_event = room_state.get((EventTypes.JoinRules, "")) + join_rules_event = await self.store.get_current_state_event( + room_id, EventTypes.JoinRules, "" + ) if join_rules_event: if not (join_rules_event.content.get("join_rule") == JoinRules.PUBLIC): # update_membership with an action of "invite" can raise a diff --git a/synapse/rest/client/room.py b/synapse/rest/client/room.py index 5a2361a2e691..dc8722067e69 100644 --- a/synapse/rest/client/room.py +++ b/synapse/rest/client/room.py @@ -673,7 +673,7 @@ async def on_GET( if include_unredacted_content and not await self.auth.is_server_admin( requester.user ): - power_level_event = await self._state.get_current_state( + power_level_event = await self._store.get_current_state_event( room_id, EventTypes.PowerLevels, "" ) diff --git a/synapse/server_notices/resource_limits_server_notices.py b/synapse/server_notices/resource_limits_server_notices.py index b5f3a0c74e9e..f904e39e2729 100644 --- a/synapse/server_notices/resource_limits_server_notices.py +++ b/synapse/server_notices/resource_limits_server_notices.py @@ -178,8 +178,8 @@ async def _is_room_currently_blocked(self, room_id: str) -> Tuple[bool, List[str currently_blocked = False pinned_state_event = None try: - pinned_state_event = await self._state.get_current_state( - room_id, event_type=EventTypes.Pinned + pinned_state_event = await self._store.get_current_state_event( + room_id, event_type=EventTypes.Pinned, state_key="" ) except AuthError: # The user has yet to join the server notices room diff --git a/tests/handlers/test_directory.py b/tests/handlers/test_directory.py index 11ad44223d39..83a661c6b08a 100644 --- a/tests/handlers/test_directory.py +++ b/tests/handlers/test_directory.py @@ -335,7 +335,7 @@ def _set_canonical_alias(self, content) -> None: def _get_canonical_alias(self): """Get the canonical alias state of the room.""" return self.get_success( - self.state_handler.get_current_state( + self.store.get_current_state_event( self.room_id, EventTypes.CanonicalAlias, "" ) ) diff --git a/tests/storage/test_purge.py b/tests/storage/test_purge.py index 08cc60237ec1..5144f70af6c7 100644 --- a/tests/storage/test_purge.py +++ b/tests/storage/test_purge.py @@ -98,9 +98,8 @@ def test_purge_room(self): first = self.helper.send(self.room_id, body="test1") # Get the current room state. - state_handler = self.hs.get_state_handler() create_event = self.get_success( - state_handler.get_current_state(self.room_id, "m.room.create", "") + self.store.get_current_state_event(self.room_id, "m.room.create", "") ) self.assertIsNotNone(create_event) From 151cb6e2f4c114c605e5f4192adf2adc79f13a79 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 20 May 2022 12:47:43 +0100 Subject: [PATCH 04/13] Use new store.get_current_state_event --- synapse/federation/federation_server.py | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/synapse/federation/federation_server.py b/synapse/federation/federation_server.py index 884b5d60b4f9..8c3fc7a45755 100644 --- a/synapse/federation/federation_server.py +++ b/synapse/federation/federation_server.py @@ -1167,14 +1167,10 @@ async def check_server_matches_acl(self, server_name: str, room_id: str) -> None Raises: AuthError if the server does not match the ACL """ - state_ids = await self.store.get_current_state_ids(room_id) - acl_event_id = state_ids.get((EventTypes.ServerACL, "")) - - if not acl_event_id: - return - - acl_event = await self.store.get_event(acl_event_id) - if server_matches_acl_event(server_name, acl_event): + acl_event = await self.store.get_current_state_event( + room_id, EventTypes.ServerACL, "" + ) + if not acl_event or server_matches_acl_event(server_name, acl_event): return raise AuthError(code=403, msg="Server is banned from room") From f69785e875872ce2b8a96c4e5b55ca17ef862438 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 20 May 2022 12:32:56 +0100 Subject: [PATCH 05/13] Add helper methods to store --- synapse/storage/databases/main/state.py | 30 +++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/synapse/storage/databases/main/state.py b/synapse/storage/databases/main/state.py index c81fadbe9e81..b8a277ae3114 100644 --- a/synapse/storage/databases/main/state.py +++ b/synapse/storage/databases/main/state.py @@ -218,6 +218,20 @@ def _get_current_state_ids_txn(txn: LoggingTransaction) -> StateMap[str]: "get_current_state_ids", _get_current_state_ids_txn ) + async def get_current_state(self, room_id: str) -> StateMap[EventBase]: + """Same as `get_current_state_ids` but also fetches the events""" + state_map_ids = await self.get_current_state_ids(room_id) + + event_map = await self.get_events(list(state_map_ids.values())) + + state_map = {} + for key, event_id in state_map_ids.items(): + event = event_map.get(event_id) + if event: + state_map[key] = event + + return state_map + # FIXME: how should this be cached? async def get_filtered_current_state_ids( self, room_id: str, state_filter: Optional[StateFilter] = None @@ -269,6 +283,22 @@ def _get_filtered_current_state_ids_txn( "get_filtered_current_state_ids", _get_filtered_current_state_ids_txn ) + async def get_filtered_current_state( + self, room_id: str, state_filter: Optional[StateFilter] = None + ) -> StateMap[EventBase]: + """Same as `get_filtered_current_state_ids` but also fetches the events""" + state_map_ids = await self.get_filtered_current_state_ids(room_id, state_filter) + + event_map = await self.get_events(list(state_map_ids.values())) + + state_map = {} + for key, event_id in state_map_ids.items(): + event = event_map.get(event_id) + if event: + state_map[key] = event + + return state_map + async def get_current_state_event( self, room_id: str, event_type: str, state_key: str ) -> Optional[EventBase]: From 11efe7231f4c5320493460ec41f4e9773c6ca120 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 20 May 2022 12:33:21 +0100 Subject: [PATCH 06/13] Use new helper functions --- synapse/handlers/federation.py | 23 +++++++++++++--------- synapse/handlers/federation_event.py | 6 +++--- synapse/handlers/initial_sync.py | 4 ++-- synapse/handlers/room.py | 2 +- synapse/handlers/room_member.py | 14 ++++++++++++- synapse/handlers/search.py | 2 +- synapse/rest/admin/rooms.py | 23 ++++++++++++++++------ tests/federation/test_federation_server.py | 4 ++-- tests/storage/test_events.py | 14 ++++++------- 9 files changed, 60 insertions(+), 32 deletions(-) diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index 0386d0a07bba..3843c304f714 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -20,7 +20,16 @@ import logging from enum import Enum from http import HTTPStatus -from typing import TYPE_CHECKING, Dict, Iterable, List, Optional, Tuple, Union +from typing import ( + TYPE_CHECKING, + Collection, + Dict, + Iterable, + List, + Optional, + Tuple, + Union, +) import attr from signedjson.key import decode_verify_key_bytes @@ -353,15 +362,11 @@ async def _maybe_backfill_inner( # First we try hosts that are already in the room # TODO: HEURISTIC ALERT. - curr_state = await self.state_handler.get_current_state(room_id) - - curr_domains = get_domains_from_state(curr_state) - - likely_domains = [ - domain for domain, depth in curr_domains if domain != self.server_name - ] + users_in_room = await self.store.get_users_in_room(room_id) + likely_domains = {get_domain_from_id(u) for u in users_in_room} + likely_domains.discard(self.server_name) - async def try_backfill(domains: List[str]) -> bool: + async def try_backfill(domains: Collection[str]) -> bool: # TODO: Should we try multiple of these at a time? for dom in domains: try: diff --git a/synapse/handlers/federation_event.py b/synapse/handlers/federation_event.py index 05c122f22491..383242a4c91e 100644 --- a/synapse/handlers/federation_event.py +++ b/synapse/handlers/federation_event.py @@ -1558,9 +1558,9 @@ async def _maybe_kick_guest_users(self, event: EventBase) -> None: if guest_access == GuestAccess.CAN_JOIN: return - current_state_map = await self._state_handler.get_current_state(event.room_id) - current_state = list(current_state_map.values()) - await self._get_room_member_handler().kick_guest_users(current_state) + current_state = await self._store.get_current_state(event.room_id) + current_state_list = list(current_state.values()) + await self._get_room_member_handler().kick_guest_users(current_state_list) async def _check_for_soft_fail( self, diff --git a/synapse/handlers/initial_sync.py b/synapse/handlers/initial_sync.py index d79248ad905b..7e6fc97e3856 100644 --- a/synapse/handlers/initial_sync.py +++ b/synapse/handlers/initial_sync.py @@ -190,7 +190,7 @@ async def handle_room(event: RoomsForUser) -> None: if event.membership == Membership.JOIN: room_end_token = now_token.room_key deferred_room_state = run_in_background( - self.state_handler.get_current_state, event.room_id + self.store.get_current_state, event.room_id ) elif event.membership == Membership.LEAVE: room_end_token = RoomStreamToken( @@ -404,7 +404,7 @@ async def _room_initial_sync_joined( membership: str, is_peeking: bool, ) -> JsonDict: - current_state = await self.state.get_current_state(room_id=room_id) + current_state = await self.store.get_current_state(room_id=room_id) # TODO: These concurrently time_now = self.clock.time_msec() diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index 92e1de050071..dca240bba4b4 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -1399,7 +1399,7 @@ async def get_event_for_timestamp( ) # Find other homeservers from the given state in the room - curr_state = await self.state_handler.get_current_state(room_id) + curr_state = await self.store.get_current_state(room_id) curr_domains = get_domains_from_state(curr_state) likely_domains = [ domain for domain, depth in curr_domains if domain != self.server_name diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py index 8d6b255cf679..85fc1aedf35c 100644 --- a/synapse/handlers/room_member.py +++ b/synapse/handlers/room_member.py @@ -1409,7 +1409,19 @@ async def _make_and_store_3pid_invite( txn_id: Optional[str], id_access_token: Optional[str] = None, ) -> int: - room_state = await self.state_handler.get_current_state(room_id) + room_state = await self.store.get_filtered_current_state( + room_id, + StateFilter.from_types( + [ + (EventTypes.Member, user.to_string()), + (EventTypes.CanonicalAlias, ""), + (EventTypes.Name, ""), + (EventTypes.Create, ""), + (EventTypes.JoinRules, ""), + (EventTypes.RoomAvatar, ""), + ] + ), + ) inviter_display_name = "" inviter_avatar_url = "" diff --git a/synapse/handlers/search.py b/synapse/handlers/search.py index cd1c47dae8b1..604b8b49446b 100644 --- a/synapse/handlers/search.py +++ b/synapse/handlers/search.py @@ -348,7 +348,7 @@ async def _search( state_results = {} if include_state: for room_id in {e.room_id for e in search_result.allowed_events}: - state = await self.state_handler.get_current_state(room_id) + state = await self.store.get_current_state(room_id) state_results[room_id] = list(state.values()) aggregations = await self._relations_handler.get_bundled_aggregations( diff --git a/synapse/rest/admin/rooms.py b/synapse/rest/admin/rooms.py index b05bda04cbf7..8c6b3d7fe7ed 100644 --- a/synapse/rest/admin/rooms.py +++ b/synapse/rest/admin/rooms.py @@ -34,6 +34,7 @@ assert_user_is_admin, ) from synapse.storage.databases.main.room import RoomSortOrder +from synapse.storage.state import StateFilter from synapse.types import JsonDict, RoomID, UserID, create_requester from synapse.util import json_decoder @@ -553,12 +554,22 @@ async def on_POST( user_to_add = content.get("user_id", requester.user.to_string()) # Figure out which local users currently have power in the room, if any. - room_state = await self.state_handler.get_current_state(room_id) - if not room_state: + filtered_room_state = await self.store.get_filtered_current_state( + room_id, + StateFilter.from_types( + [ + (EventTypes.Create, ""), + (EventTypes.PowerLevels, ""), + (EventTypes.JoinRules, ""), + (EventTypes.Member, user_to_add), + ] + ), + ) + if not filtered_room_state: raise SynapseError(HTTPStatus.BAD_REQUEST, "Server not in room") - create_event = room_state[(EventTypes.Create, "")] - power_levels = room_state.get((EventTypes.PowerLevels, "")) + create_event = filtered_room_state[(EventTypes.Create, "")] + power_levels = filtered_room_state.get((EventTypes.PowerLevels, "")) if power_levels is not None: # We pick the local user with the highest power. @@ -634,7 +645,7 @@ async def on_POST( # Now we check if the user we're granting admin rights to is already in # the room. If not and it's not a public room we invite them. - member_event = room_state.get((EventTypes.Member, user_to_add)) + member_event = filtered_room_state.get((EventTypes.Member, user_to_add)) is_joined = False if member_event: is_joined = member_event.content["membership"] in ( @@ -645,7 +656,7 @@ async def on_POST( if is_joined: return HTTPStatus.OK, {} - join_rules = room_state.get((EventTypes.JoinRules, "")) + join_rules = filtered_room_state.get((EventTypes.JoinRules, "")) is_public = False if join_rules: is_public = join_rules.content.get("join_rule") == JoinRules.PUBLIC diff --git a/tests/federation/test_federation_server.py b/tests/federation/test_federation_server.py index b19365b81a4c..0f9fb0abf18b 100644 --- a/tests/federation/test_federation_server.py +++ b/tests/federation/test_federation_server.py @@ -207,7 +207,7 @@ def test_send_join(self): # the room should show that the new user is a member r = self.get_success( - self.hs.get_state_handler().get_current_state(self._room_id) + self.hs.get_datastores().main.get_current_state(self._room_id) ) self.assertEqual(r[("m.room.member", joining_user)].membership, "join") @@ -258,7 +258,7 @@ def test_send_join_partial_state(self): # the room should show that the new user is a member r = self.get_success( - self.hs.get_state_handler().get_current_state(self._room_id) + self.hs.get_datastores().main.get_current_state(self._room_id) ) self.assertEqual(r[("m.room.member", joining_user)].membership, "join") diff --git a/tests/storage/test_events.py b/tests/storage/test_events.py index ef5e25873c22..18a5907dab21 100644 --- a/tests/storage/test_events.py +++ b/tests/storage/test_events.py @@ -103,7 +103,7 @@ def test_prune_gap(self): RoomVersions.V6, ) - state_before_gap = self.get_success(self.state.get_current_state(self.room_id)) + state_before_gap = self.get_success(self.store.get_current_state(self.room_id)) self.persist_event(remote_event_2, state=state_before_gap.values()) @@ -135,7 +135,7 @@ def test_do_not_prune_gap_if_state_different(self): # setting. The state resolution across the old and new event will then # include it, and so the resolved state won't match the new state. state_before_gap = dict( - self.get_success(self.state.get_current_state(self.room_id)) + self.get_success(self.store.get_current_state(self.room_id)) ) state_before_gap.pop(("m.room.history_visibility", "")) @@ -177,7 +177,7 @@ def test_prune_gap_if_old(self): RoomVersions.V6, ) - state_before_gap = self.get_success(self.state.get_current_state(self.room_id)) + state_before_gap = self.get_success(self.store.get_current_state(self.room_id)) self.persist_event(remote_event_2, state=state_before_gap.values()) @@ -207,7 +207,7 @@ def test_do_not_prune_gap_if_other_server(self): RoomVersions.V6, ) - state_before_gap = self.get_success(self.state.get_current_state(self.room_id)) + state_before_gap = self.get_success(self.store.get_current_state(self.room_id)) self.persist_event(remote_event_2, state=state_before_gap.values()) @@ -247,7 +247,7 @@ def test_prune_gap_if_dummy_remote(self): RoomVersions.V6, ) - state_before_gap = self.get_success(self.state.get_current_state(self.room_id)) + state_before_gap = self.get_success(self.store.get_current_state(self.room_id)) self.persist_event(remote_event_2, state=state_before_gap.values()) @@ -289,7 +289,7 @@ def test_prune_gap_if_dummy_local(self): RoomVersions.V6, ) - state_before_gap = self.get_success(self.state.get_current_state(self.room_id)) + state_before_gap = self.get_success(self.store.get_current_state(self.room_id)) self.persist_event(remote_event_2, state=state_before_gap.values()) @@ -323,7 +323,7 @@ def test_do_not_prune_gap_if_not_dummy(self): RoomVersions.V6, ) - state_before_gap = self.get_success(self.state.get_current_state(self.room_id)) + state_before_gap = self.get_success(self.store.get_current_state(self.room_id)) self.persist_event(remote_event_2, state=state_before_gap.values()) From 68ff8f357566a7aa2553592c906aa4c9d53dd744 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 20 May 2022 12:33:44 +0100 Subject: [PATCH 07/13] Remove 'get_current_state' from StateHandler --- synapse/state/__init__.py | 64 --------------------------------------- 1 file changed, 64 deletions(-) diff --git a/synapse/state/__init__.py b/synapse/state/__init__.py index 4b4ed42cff33..8946f77dfd18 100644 --- a/synapse/state/__init__.py +++ b/synapse/state/__init__.py @@ -32,13 +32,11 @@ Set, Tuple, Union, - overload, ) import attr from frozendict import frozendict from prometheus_client import Counter, Histogram -from typing_extensions import Literal from synapse.api.constants import EventTypes from synapse.api.room_versions import KNOWN_ROOM_VERSIONS, StateResolutionVersions @@ -132,68 +130,6 @@ def __init__(self, hs: "HomeServer"): self._state_resolution_handler = hs.get_state_resolution_handler() self._storage = hs.get_storage() - @overload - async def get_current_state( - self, - room_id: str, - event_type: Literal[None] = None, - state_key: str = "", - latest_event_ids: Optional[List[str]] = None, - ) -> StateMap[EventBase]: - ... - - @overload - async def get_current_state( - self, - room_id: str, - event_type: str, - state_key: str = "", - latest_event_ids: Optional[List[str]] = None, - ) -> Optional[EventBase]: - ... - - async def get_current_state( - self, - room_id: str, - event_type: Optional[str] = None, - state_key: str = "", - latest_event_ids: Optional[List[str]] = None, - ) -> Union[Optional[EventBase], StateMap[EventBase]]: - """Retrieves the current state for the room. This is done by - calling `get_latest_events_in_room` to get the leading edges of the - event graph and then resolving any of the state conflicts. - - This is equivalent to getting the state of an event that were to send - next before receiving any new events. - - Returns: - If `event_type` is specified, then the method returns only the one - event (or None) with that `event_type` and `state_key`. - - Otherwise, a map from (type, state_key) to event. - """ - if not latest_event_ids: - latest_event_ids = await self.store.get_latest_event_ids_in_room(room_id) - assert latest_event_ids is not None - - logger.debug("calling resolve_state_groups from get_current_state") - ret = await self.resolve_state_groups_for_events(room_id, latest_event_ids) - state = ret.state - - if event_type: - event_id = state.get((event_type, state_key)) - event = None - if event_id: - event = await self.store.get_event(event_id, allow_none=True) - return event - - state_map = await self.store.get_events( - list(state.values()), get_prev_content=False - ) - return { - key: state_map[e_id] for key, e_id in state.items() if e_id in state_map - } - async def get_current_state_ids( self, room_id: str, latest_event_ids: Optional[Collection[str]] = None ) -> StateMap[str]: From 9bb3bbe1530b203243f79b44ed84b438945f0923 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 20 May 2022 12:47:52 +0100 Subject: [PATCH 08/13] Require 'latest_event_ids' --- synapse/state/__init__.py | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/synapse/state/__init__.py b/synapse/state/__init__.py index 8946f77dfd18..347fa96ce7ca 100644 --- a/synapse/state/__init__.py +++ b/synapse/state/__init__.py @@ -131,22 +131,19 @@ def __init__(self, hs: "HomeServer"): self._storage = hs.get_storage() async def get_current_state_ids( - self, room_id: str, latest_event_ids: Optional[Collection[str]] = None + self, + room_id: str, + latest_event_ids: Collection[str], ) -> StateMap[str]: """Get the current state, or the state at a set of events, for a room Args: room_id: - latest_event_ids: if given, the forward extremities to resolve. If - None, we look them up from the database (via a cache). + latest_event_ids: The forward extremities to resolve. Returns: the state dict, mapping from (event_type, state_key) -> event_id """ - if not latest_event_ids: - latest_event_ids = await self.store.get_latest_event_ids_in_room(room_id) - assert latest_event_ids is not None - logger.debug("calling resolve_state_groups from get_current_state_ids") ret = await self.resolve_state_groups_for_events(room_id, latest_event_ids) return ret.state From c8c12ac13ae383298e8bc922f6279ce458492ba1 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 20 May 2022 12:55:53 +0100 Subject: [PATCH 09/13] Pull out less state when checking soft fail --- synapse/handlers/federation_event.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/synapse/handlers/federation_event.py b/synapse/handlers/federation_event.py index 383242a4c91e..3c7ae2ea39c9 100644 --- a/synapse/handlers/federation_event.py +++ b/synapse/handlers/federation_event.py @@ -1588,6 +1588,9 @@ async def _check_for_soft_fail( room_version = await self._store.get_room_version_id(event.room_id) room_version_obj = KNOWN_ROOM_VERSIONS[room_version] + # The event types we want to pull from the "current" state. + auth_types = auth_types_for_event(room_version_obj, event) + # Calculate the "current state". if state is not None: # If we're explicitly given the state then we won't have all the @@ -1614,8 +1617,8 @@ async def _check_for_soft_fail( k: e.event_id for k, e in current_states.items() } else: - current_state_ids = await self._state_handler.get_current_state_ids( - event.room_id, latest_event_ids=extrem_ids + current_state_ids = await self._store.get_filtered_current_state_ids( + event.room_id, StateFilter.from_types(auth_types) ) logger.debug( @@ -1625,7 +1628,6 @@ async def _check_for_soft_fail( ) # Now check if event pass auth against said current state - auth_types = auth_types_for_event(room_version_obj, event) current_state_ids_list = [ e for k, e in current_state_ids.items() if k in auth_types ] From 4bd06c9c986fec2791983228212f8367c28de9d2 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 20 May 2022 13:02:47 +0100 Subject: [PATCH 10/13] Newsfile --- changelog.d/12811.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/12811.misc diff --git a/changelog.d/12811.misc b/changelog.d/12811.misc new file mode 100644 index 000000000000..d57e1aca6bf0 --- /dev/null +++ b/changelog.d/12811.misc @@ -0,0 +1 @@ +Reduce the amount of state we pull from the DB. From 456a394bf7ee3e802055eb38c0079830df3989df Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 20 May 2022 15:13:06 +0100 Subject: [PATCH 11/13] Use 'get_domains_from_state' still for backfill --- synapse/handlers/federation.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index 3843c304f714..287ed9b2e853 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -362,11 +362,15 @@ async def _maybe_backfill_inner( # First we try hosts that are already in the room # TODO: HEURISTIC ALERT. - users_in_room = await self.store.get_users_in_room(room_id) - likely_domains = {get_domain_from_id(u) for u in users_in_room} - likely_domains.discard(self.server_name) + curr_state = await self.store.get_current_state(room_id) - async def try_backfill(domains: Collection[str]) -> bool: + curr_domains = get_domains_from_state(curr_state) + + likely_domains = [ + domain for domain, depth in curr_domains if domain != self.server_name + ] + + async def try_backfill(domains: List[str]) -> bool: # TODO: Should we try multiple of these at a time? for dom in domains: try: From 2dd2ca17a0310990ae2db277ef3e6ea67b46ecad Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 20 May 2022 18:34:05 +0100 Subject: [PATCH 12/13] Fix lint --- synapse/handlers/federation.py | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index 287ed9b2e853..1fab49fe8215 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -20,16 +20,7 @@ import logging from enum import Enum from http import HTTPStatus -from typing import ( - TYPE_CHECKING, - Collection, - Dict, - Iterable, - List, - Optional, - Tuple, - Union, -) +from typing import TYPE_CHECKING, Dict, Iterable, List, Optional, Tuple, Union import attr from signedjson.key import decode_verify_key_bytes From ed65e4afbab3a36f3bb147efe15f77fd86c17915 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 23 May 2022 11:06:24 +0100 Subject: [PATCH 13/13] Remove unused param in `check_user_in_room` --- synapse/api/auth.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/synapse/api/auth.py b/synapse/api/auth.py index 83a30fe7ad96..2fe0939e9dd7 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -29,12 +29,11 @@ MissingClientTokenError, ) from synapse.appservice import ApplicationService -from synapse.events import EventBase from synapse.http import get_request_user_agent from synapse.http.site import SynapseRequest from synapse.logging.opentracing import active_span, force_tracing, start_active_span from synapse.storage.databases.main.registration import TokenLookupResult -from synapse.types import Requester, StateMap, UserID, create_requester +from synapse.types import Requester, UserID, create_requester from synapse.util.caches.lrucache import LruCache from synapse.util.macaroons import get_value_from_macaroon, satisfy_expiry @@ -78,7 +77,6 @@ async def check_user_in_room( self, room_id: str, user_id: str, - current_state: Optional[StateMap[EventBase]] = None, allow_departed_users: bool = False, ) -> Tuple[str, Optional[str]]: """Check if the user is in the room, or was at some point.