From a2c36e9277bd78a01c88baec760ea56e07006184 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 25 Nov 2020 15:04:46 +0000 Subject: [PATCH 1/6] Add a new store method for retrieving the local current membership of a user --- synapse/storage/databases/main/roommember.py | 47 +++++++++++++++++++- 1 file changed, 46 insertions(+), 1 deletion(-) diff --git a/synapse/storage/databases/main/roommember.py b/synapse/storage/databases/main/roommember.py index 01d9dbb36f44..63264acd5c95 100644 --- a/synapse/storage/databases/main/roommember.py +++ b/synapse/storage/databases/main/roommember.py @@ -14,7 +14,7 @@ # See the License for the specific language governing permissions and # limitations under the License. import logging -from typing import TYPE_CHECKING, Dict, FrozenSet, Iterable, List, Optional, Set +from typing import TYPE_CHECKING, Dict, FrozenSet, Iterable, List, Optional, Set, Tuple from synapse.api.constants import EventTypes, Membership from synapse.events import EventBase @@ -350,6 +350,51 @@ def _get_rooms_for_local_user_where_membership_is_txn( return results + @cached() + async def get_local_current_membership_for_user_in_room( + self, user_id: str, room_id: str + ) -> Tuple[Optional[str], Optional[str]]: + """Retrieve the current local membership state and event ID for a user in a room. + + Args: + user_id: The ID of the user. + room_id: The ID of the room. + + Returns: + A tuple of (membership_type, event_id). Both will be None if a + room_id/user_id pair is not found. + + """ + return await self.db_pool.runInteraction( + "get_local_current_membership_for_user_in_room", + self._get_local_current_membership_for_user_in_room_txn, + user_id, + room_id, + ) + + def _get_local_current_membership_for_user_in_room_txn( + self, txn, user_id: str, room_id: str + ) -> Tuple[Optional[str], Optional[str]]: + # Paranoia check. + if not self.hs.is_mine_id(user_id): + raise Exception( + "Cannot call 'get_local_current_membership_for_user_in_room' on " + "non-local user %s" % (user_id,), + ) + + sql = """ + SELECT membership, event_id + FROM local_current_membership + WHERE + room_id = ? AND user_id = ? + """ + + txn.execute(sql, (room_id, user_id)) + row = txn.fetchone() + if row: + return row[0], row[1] + return None, None + @cached(max_entries=500000, iterable=True) async def get_rooms_for_user_with_stream_ordering( self, user_id: str From 5ef9d9ca7d48531bf628ba1023c7ba49a7d915d9 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 25 Nov 2020 15:10:10 +0000 Subject: [PATCH 2/6] Update invite rejection code to use the new store method --- synapse/handlers/room_member.py | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py index 70f896626753..c6174aaeab64 100644 --- a/synapse/handlers/room_member.py +++ b/synapse/handlers/room_member.py @@ -31,7 +31,6 @@ from synapse.api.ratelimiting import Ratelimiter from synapse.events import EventBase from synapse.events.snapshot import EventContext -from synapse.storage.roommember import RoomsForUser from synapse.types import JsonDict, Requester, RoomAlias, RoomID, StateMap, UserID from synapse.util.async_helpers import Linearizer from synapse.util.distributor import user_left_room @@ -515,10 +514,17 @@ async def update_membership_locked( elif effective_membership_state == Membership.LEAVE: if not is_host_in_room: # perhaps we've been invited - invite = await self.store.get_invite_for_local_user_in_room( - user_id=target.to_string(), room_id=room_id - ) # type: Optional[RoomsForUser] - if not invite: + ( + current_membership_type, + current_membership_event_id, + ) = await self.store.get_local_current_membership_for_user_in_room( + target.to_string(), room_id + ) + if ( + not current_membership_type + or current_membership_type != Membership.INVITE + or not current_membership_event_id + ): logger.info( "%s sent a leave request to %s, but that is not an active room " "on this server, and there is no pending invite", @@ -528,6 +534,7 @@ async def update_membership_locked( raise SynapseError(404, "Not a known room") + invite = await self.store.get_event(current_membership_event_id) logger.info( "%s rejects invite to %s from %s", target, room_id, invite.sender ) From cde147b3878316c053c7312e136aa86b6ab29c84 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 25 Nov 2020 15:21:57 +0000 Subject: [PATCH 3/6] Changelog --- changelog.d/8815.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/8815.misc diff --git a/changelog.d/8815.misc b/changelog.d/8815.misc new file mode 100644 index 000000000000..647edeb56851 --- /dev/null +++ b/changelog.d/8815.misc @@ -0,0 +1 @@ +Optimise the lookup for an invite from another homeserver when trying to reject it. \ No newline at end of file From 664132c31642e063020622f60b0f7a9885ce3152 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 25 Nov 2020 16:18:57 +0000 Subject: [PATCH 4/6] Use simple_select_one instead of custom txn function --- synapse/storage/databases/main/roommember.py | 32 ++++++-------------- 1 file changed, 10 insertions(+), 22 deletions(-) diff --git a/synapse/storage/databases/main/roommember.py b/synapse/storage/databases/main/roommember.py index 63264acd5c95..7fc22252165d 100644 --- a/synapse/storage/databases/main/roommember.py +++ b/synapse/storage/databases/main/roommember.py @@ -363,18 +363,7 @@ async def get_local_current_membership_for_user_in_room( Returns: A tuple of (membership_type, event_id). Both will be None if a room_id/user_id pair is not found. - """ - return await self.db_pool.runInteraction( - "get_local_current_membership_for_user_in_room", - self._get_local_current_membership_for_user_in_room_txn, - user_id, - room_id, - ) - - def _get_local_current_membership_for_user_in_room_txn( - self, txn, user_id: str, room_id: str - ) -> Tuple[Optional[str], Optional[str]]: # Paranoia check. if not self.hs.is_mine_id(user_id): raise Exception( @@ -382,18 +371,17 @@ def _get_local_current_membership_for_user_in_room_txn( "non-local user %s" % (user_id,), ) - sql = """ - SELECT membership, event_id - FROM local_current_membership - WHERE - room_id = ? AND user_id = ? - """ + results_dict = await self.db_pool.simple_select_one( + "local_current_membership", + {"room_id": room_id, "user_id": user_id}, + ("membership", "event_id"), + allow_none=True, + desc="get_local_current_membership_for_user_in_room", + ) + if not results_dict: + return None, None - txn.execute(sql, (room_id, user_id)) - row = txn.fetchone() - if row: - return row[0], row[1] - return None, None + return results_dict.get("membership"), results_dict.get("event_id") @cached(max_entries=500000, iterable=True) async def get_rooms_for_user_with_stream_ordering( From a584963b5107432e3838ef2d8ed7b9a5db36dda2 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 25 Nov 2020 19:20:46 +0000 Subject: [PATCH 5/6] Simplify conditional --- synapse/handlers/room_member.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py index c6174aaeab64..13a793c05a5a 100644 --- a/synapse/handlers/room_member.py +++ b/synapse/handlers/room_member.py @@ -521,8 +521,7 @@ async def update_membership_locked( target.to_string(), room_id ) if ( - not current_membership_type - or current_membership_type != Membership.INVITE + current_membership_type != Membership.INVITE or not current_membership_event_id ): logger.info( From 264823685f67a2a5860212522fd8bbe7fd779e31 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 25 Nov 2020 19:34:57 +0000 Subject: [PATCH 6/6] Remove cache, #sadtimes --- synapse/storage/databases/main/roommember.py | 1 - 1 file changed, 1 deletion(-) diff --git a/synapse/storage/databases/main/roommember.py b/synapse/storage/databases/main/roommember.py index 7fc22252165d..dcdaf09682b6 100644 --- a/synapse/storage/databases/main/roommember.py +++ b/synapse/storage/databases/main/roommember.py @@ -350,7 +350,6 @@ def _get_rooms_for_local_user_where_membership_is_txn( return results - @cached() async def get_local_current_membership_for_user_in_room( self, user_id: str, room_id: str ) -> Tuple[Optional[str], Optional[str]]: