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

Commit

Permalink
Don't remove local users from dir when the leave their last room (#11103
Browse files Browse the repository at this point in the history
)
  • Loading branch information
David Robertson committed Oct 18, 2021
1 parent e09be0c commit 37b845d
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 5 deletions.
1 change: 1 addition & 0 deletions changelog.d/11103.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix local users who left all their rooms being removed from the user directory, even if the "search_all_users" config option was enabled.
13 changes: 8 additions & 5 deletions synapse/handlers/user_directory.py
Original file line number Diff line number Diff line change
Expand Up @@ -415,16 +415,19 @@ async def _handle_remove_user(self, room_id: str, user_id: str) -> None:
room_id: The room ID that user left or stopped being public that
user_id
"""
logger.debug("Removing user %r", user_id)
logger.debug("Removing user %r from room %r", user_id, room_id)

# Remove user from sharing tables
await self.store.remove_user_who_share_room(user_id, room_id)

# Are they still in any rooms? If not, remove them entirely.
rooms_user_is_in = await self.store.get_user_dir_rooms_user_is_in(user_id)
# Additionally, if they're a remote user and we're no longer joined
# to any rooms they're in, remove them from the user directory.
if not self.is_mine_id(user_id):
rooms_user_is_in = await self.store.get_user_dir_rooms_user_is_in(user_id)

if len(rooms_user_is_in) == 0:
await self.store.remove_from_user_dir(user_id)
if len(rooms_user_is_in) == 0:
logger.debug("Removing user %r from directory", user_id)
await self.store.remove_from_user_dir(user_id)

async def _handle_possible_remote_profile_change(
self,
Expand Down
50 changes: 50 additions & 0 deletions tests/handlers/test_user_directory.py
Original file line number Diff line number Diff line change
Expand Up @@ -914,6 +914,56 @@ def _add_user_to_room(
self.hs.get_storage().persistence.persist_event(event, context)
)

def test_local_user_leaving_room_remains_in_user_directory(self) -> None:
"""We've chosen to simplify the user directory's implementation by
always including local users. Ensure this invariant is maintained when
a local user
- leaves a room, and
- leaves the last room they're in which is visible to this server.
This is user-visible if the "search_all_users" config option is on: the
local user who left a room would no longer be searchable if this test fails!
"""
alice = self.register_user("alice", "pass")
alice_token = self.login(alice, "pass")
bob = self.register_user("bob", "pass")
bob_token = self.login(bob, "pass")

# Alice makes two public rooms, which Bob joins.
room1 = self.helper.create_room_as(alice, is_public=True, tok=alice_token)
room2 = self.helper.create_room_as(alice, is_public=True, tok=alice_token)
self.helper.join(room1, bob, tok=bob_token)
self.helper.join(room2, bob, tok=bob_token)

# The user directory tables are updated.
users, in_public, in_private = self.get_success(
self.user_dir_helper.get_tables()
)
self.assertEqual(users, {alice, bob})
self.assertEqual(
in_public, {(alice, room1), (alice, room2), (bob, room1), (bob, room2)}
)
self.assertEqual(in_private, set())

# Alice leaves one room. She should still be in the directory.
self.helper.leave(room1, alice, tok=alice_token)
users, in_public, in_private = self.get_success(
self.user_dir_helper.get_tables()
)
self.assertEqual(users, {alice, bob})
self.assertEqual(in_public, {(alice, room2), (bob, room1), (bob, room2)})
self.assertEqual(in_private, set())

# Alice leaves the other. She should still be in the directory.
self.helper.leave(room2, alice, tok=alice_token)
self.wait_for_background_updates()
users, in_public, in_private = self.get_success(
self.user_dir_helper.get_tables()
)
self.assertEqual(users, {alice, bob})
self.assertEqual(in_public, {(bob, room1), (bob, room2)})
self.assertEqual(in_private, set())


class TestUserDirSearchDisabled(unittest.HomeserverTestCase):
servlets = [
Expand Down

0 comments on commit 37b845d

Please sign in to comment.