From 94f40e54d235c2bcea796953d9e58ee4f4c21344 Mon Sep 17 00:00:00 2001 From: Jonathan de Jong Date: Sun, 22 May 2022 17:51:52 +0200 Subject: [PATCH 1/7] Remove dependency on user directory for mutual rooms --- synapse/rest/client/mutual_rooms.py | 15 +------ synapse/storage/databases/main/roommember.py | 22 ++++++++++ .../storage/databases/main/user_directory.py | 43 ------------------- tests/rest/client/test_mutual_rooms.py | 2 - 4 files changed, 24 insertions(+), 58 deletions(-) diff --git a/synapse/rest/client/mutual_rooms.py b/synapse/rest/client/mutual_rooms.py index 27bfaf0b2936..38ef4e459f77 100644 --- a/synapse/rest/client/mutual_rooms.py +++ b/synapse/rest/client/mutual_rooms.py @@ -42,21 +42,10 @@ def __init__(self, hs: "HomeServer"): super().__init__() self.auth = hs.get_auth() self.store = hs.get_datastores().main - self.user_directory_search_enabled = ( - hs.config.userdirectory.user_directory_search_enabled - ) async def on_GET( self, request: SynapseRequest, user_id: str ) -> Tuple[int, JsonDict]: - - if not self.user_directory_search_enabled: - raise SynapseError( - code=400, - msg="User directory searching is disabled. Cannot determine shared rooms.", - errcode=Codes.UNKNOWN, - ) - UserID.from_string(user_id) requester = await self.auth.get_user_by_req(request) @@ -67,8 +56,8 @@ async def on_GET( errcode=Codes.FORBIDDEN, ) - rooms = await self.store.get_mutual_rooms_for_users( - requester.user.to_string(), user_id + rooms = await self.store.get_mutual_rooms_between_users( + frozenset((requester.user.to_string(), user_id)) ) return 200, {"joined": list(rooms)} diff --git a/synapse/storage/databases/main/roommember.py b/synapse/storage/databases/main/roommember.py index cc528fcf2dae..1156daad4ae9 100644 --- a/synapse/storage/databases/main/roommember.py +++ b/synapse/storage/databases/main/roommember.py @@ -670,6 +670,28 @@ async def get_users_who_share_room_with_user( return user_who_share_room + @cached( + max_entries=500000, + cache_context=True, + iterable=True, + prune_unread_entries=False, + ) + async def get_mutual_rooms_between_users( + self, user_ids: FrozenSet[str], cache_context: _CacheContext + ) -> Set[str]: + """Returns the set of rooms that all users in user_ids share""" + shared_room_ids = None + for user_id in user_ids: + room_ids = await self.get_rooms_for_user( + user_id, on_invalidate=cache_context.invalidate + ) + if shared_room_ids is None: + shared_room_ids = room_ids + else: + shared_room_ids &= room_ids + + return shared_room_ids or set() + async def get_joined_users_from_context( self, event: EventBase, context: EventContext ) -> Dict[str, ProfileInfo]: diff --git a/synapse/storage/databases/main/user_directory.py b/synapse/storage/databases/main/user_directory.py index 028db69af301..2282242e9d17 100644 --- a/synapse/storage/databases/main/user_directory.py +++ b/synapse/storage/databases/main/user_directory.py @@ -729,49 +729,6 @@ async def get_user_dir_rooms_user_is_in(self, user_id: str) -> List[str]: users.update(rows) return list(users) - async def get_mutual_rooms_for_users( - self, user_id: str, other_user_id: str - ) -> Set[str]: - """ - Returns the rooms that a local user shares with another local or remote user. - - Args: - user_id: The MXID of a local user - other_user_id: The MXID of the other user - - Returns: - A set of room ID's that the users share. - """ - - def _get_mutual_rooms_for_users_txn( - txn: LoggingTransaction, - ) -> List[Dict[str, str]]: - txn.execute( - """ - SELECT p1.room_id - FROM users_in_public_rooms as p1 - INNER JOIN users_in_public_rooms as p2 - ON p1.room_id = p2.room_id - AND p1.user_id = ? - AND p2.user_id = ? - UNION - SELECT room_id - FROM users_who_share_private_rooms - WHERE - user_id = ? - AND other_user_id = ? - """, - (user_id, other_user_id, user_id, other_user_id), - ) - rows = self.db_pool.cursor_to_dict(txn) - return rows - - rows = await self.db_pool.runInteraction( - "get_mutual_rooms_for_users", _get_mutual_rooms_for_users_txn - ) - - return {row["room_id"] for row in rows} - async def get_user_directory_stream_pos(self) -> Optional[int]: """ Get the stream ID of the user directory stream. diff --git a/tests/rest/client/test_mutual_rooms.py b/tests/rest/client/test_mutual_rooms.py index 7b7d283bb628..a4327f7aceff 100644 --- a/tests/rest/client/test_mutual_rooms.py +++ b/tests/rest/client/test_mutual_rooms.py @@ -36,12 +36,10 @@ class UserMutualRoomsTest(unittest.HomeserverTestCase): def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer: config = self.default_config() - config["update_user_directory"] = True return self.setup_test_homeserver(config=config) def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: self.store = hs.get_datastores().main - self.handler = hs.get_user_directory_handler() def _get_mutual_rooms(self, token: str, other_user: str) -> FakeChannel: return self.make_request( From 890d63e4661fbed653b948e8361d0dcca4dfca0f Mon Sep 17 00:00:00 2001 From: Jonathan de Jong Date: Sun, 22 May 2022 17:55:42 +0200 Subject: [PATCH 2/7] news --- changelog.d/12836.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/12836.misc diff --git a/changelog.d/12836.misc b/changelog.d/12836.misc new file mode 100644 index 000000000000..e2a5dc982a59 --- /dev/null +++ b/changelog.d/12836.misc @@ -0,0 +1 @@ +Remove Mutual Rooms (MSC2666) endpoint dependency on the User Directory. \ No newline at end of file From fc51b3665cfffef228032e5fa7229fea6768c3cd Mon Sep 17 00:00:00 2001 From: Jonathan de Jong Date: Sun, 22 May 2022 18:14:17 +0200 Subject: [PATCH 3/7] fix lint --- synapse/storage/databases/main/roommember.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/synapse/storage/databases/main/roommember.py b/synapse/storage/databases/main/roommember.py index 1156daad4ae9..c307c1872c63 100644 --- a/synapse/storage/databases/main/roommember.py +++ b/synapse/storage/databases/main/roommember.py @@ -678,19 +678,19 @@ async def get_users_who_share_room_with_user( ) async def get_mutual_rooms_between_users( self, user_ids: FrozenSet[str], cache_context: _CacheContext - ) -> Set[str]: + ) -> FrozenSet[str]: """Returns the set of rooms that all users in user_ids share""" - shared_room_ids = None + shared_room_ids: Optional[FrozenSet[str]] = None for user_id in user_ids: room_ids = await self.get_rooms_for_user( user_id, on_invalidate=cache_context.invalidate ) - if shared_room_ids is None: - shared_room_ids = room_ids - else: + if shared_room_ids is not None: shared_room_ids &= room_ids + else: + shared_room_ids = room_ids - return shared_room_ids or set() + return shared_room_ids or frozenset() async def get_joined_users_from_context( self, event: EventBase, context: EventContext From 57fd2149cb2e39cb427edada253ca44251a74573 Mon Sep 17 00:00:00 2001 From: Jonathan de Jong Date: Wed, 25 May 2022 10:11:18 +0200 Subject: [PATCH 4/7] address review feedback --- synapse/storage/databases/main/roommember.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/synapse/storage/databases/main/roommember.py b/synapse/storage/databases/main/roommember.py index c307c1872c63..43f0cbd88175 100644 --- a/synapse/storage/databases/main/roommember.py +++ b/synapse/storage/databases/main/roommember.py @@ -671,15 +671,13 @@ async def get_users_who_share_room_with_user( return user_who_share_room @cached( - max_entries=500000, cache_context=True, iterable=True, - prune_unread_entries=False, ) async def get_mutual_rooms_between_users( self, user_ids: FrozenSet[str], cache_context: _CacheContext ) -> FrozenSet[str]: - """Returns the set of rooms that all users in user_ids share""" + """Returns the set of rooms that all users in `user_ids` share""" shared_room_ids: Optional[FrozenSet[str]] = None for user_id in user_ids: room_ids = await self.get_rooms_for_user( From de4a2a8c258cda3a63dbe36645845d93de50b716 Mon Sep 17 00:00:00 2001 From: Jonathan de Jong Date: Wed, 25 May 2022 11:43:24 +0200 Subject: [PATCH 5/7] add docstring for params --- synapse/storage/databases/main/roommember.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/synapse/storage/databases/main/roommember.py b/synapse/storage/databases/main/roommember.py index 43f0cbd88175..e100aa572492 100644 --- a/synapse/storage/databases/main/roommember.py +++ b/synapse/storage/databases/main/roommember.py @@ -677,7 +677,14 @@ async def get_users_who_share_room_with_user( async def get_mutual_rooms_between_users( self, user_ids: FrozenSet[str], cache_context: _CacheContext ) -> FrozenSet[str]: - """Returns the set of rooms that all users in `user_ids` share""" + """ + Returns the set of rooms that all users in `user_ids` share. + + Args: + user_ids: A frozen set of all users to investigate and return + overlapping joined rooms for. + cache_context + """ shared_room_ids: Optional[FrozenSet[str]] = None for user_id in user_ids: room_ids = await self.get_rooms_for_user( From 8bdb3d35bf4745eb4a9a5d5b20c7cbd8dee8ebe7 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 25 May 2022 08:23:09 -0400 Subject: [PATCH 6/7] Update changelog. --- changelog.d/12836.misc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/12836.misc b/changelog.d/12836.misc index e2a5dc982a59..85909c6a2d1b 100644 --- a/changelog.d/12836.misc +++ b/changelog.d/12836.misc @@ -1 +1 @@ -Remove Mutual Rooms (MSC2666) endpoint dependency on the User Directory. \ No newline at end of file +Remove Mutual Rooms ([MSC2666](https://github.com/matrix-org/matrix-spec-proposals/pull/2666)) endpoint dependency on the User Directory. \ No newline at end of file From b52518c990a6de4e770314801708eb6507a138b5 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 25 May 2022 08:24:02 -0400 Subject: [PATCH 7/7] Formatting. --- synapse/storage/databases/main/roommember.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/synapse/storage/databases/main/roommember.py b/synapse/storage/databases/main/roommember.py index e100aa572492..e222b7bd1f88 100644 --- a/synapse/storage/databases/main/roommember.py +++ b/synapse/storage/databases/main/roommember.py @@ -670,10 +670,7 @@ async def get_users_who_share_room_with_user( return user_who_share_room - @cached( - cache_context=True, - iterable=True, - ) + @cached(cache_context=True, iterable=True) async def get_mutual_rooms_between_users( self, user_ids: FrozenSet[str], cache_context: _CacheContext ) -> FrozenSet[str]: