Clarifications in user directory for users who share rooms tracking (#13966)

Spawned while working on [`get_users_in_room` mis-uses](https://github.com/matrix-org/synapse/pull/13958#discussion_r984074897) and thinking we could use `get_local_users_in_room` here but we can't.

From first glance, it seemed like this was only using local users from all of the `is_mine_id(user_id)` checks but I see that it does actually use remote users. Just making things a little more clear here what it does and mentions remote users so maybe that will be more obvious in the future.
pull/14009/head
Eric Eastwood 2022-09-30 14:40:18 -05:00 committed by GitHub
parent 535f8c8f7d
commit ad4c14e4b0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 25 additions and 12 deletions

1
changelog.d/13966.misc Normal file
View File

@ -0,0 +1 @@
Refactor language in user directory `_track_user_joined_room` code to make it more clear that we use both local and remote users.

View File

@ -13,7 +13,7 @@
# limitations under the License.
import logging
from typing import TYPE_CHECKING, Any, Dict, List, Optional
from typing import TYPE_CHECKING, Any, Dict, List, Optional, Set, Tuple
import synapse.metrics
from synapse.api.constants import EventTypes, HistoryVisibility, JoinRules, Membership
@ -379,7 +379,7 @@ class UserDirectoryHandler(StateDeltasHandler):
user_id, event.content.get("displayname"), event.content.get("avatar_url")
)
async def _track_user_joined_room(self, room_id: str, user_id: str) -> None:
async def _track_user_joined_room(self, room_id: str, joining_user_id: str) -> None:
"""Someone's just joined a room. Update `users_in_public_rooms` or
`users_who_share_private_rooms` as appropriate.
@ -390,32 +390,44 @@ class UserDirectoryHandler(StateDeltasHandler):
room_id
)
if is_public:
await self.store.add_users_in_public_rooms(room_id, (user_id,))
await self.store.add_users_in_public_rooms(room_id, (joining_user_id,))
else:
users_in_room = await self.store.get_users_in_room(room_id)
other_users_in_room = [
other
for other in users_in_room
if other != user_id
if other != joining_user_id
and (
# We can't apply any special rules to remote users so
# they're always included
not self.is_mine_id(other)
# Check the special rules whether the local user should be
# included in the user directory
or await self.store.should_include_local_user_in_dir(other)
)
]
to_insert = set()
updates_to_users_who_share_rooms: Set[Tuple[str, str]] = set()
# First, if they're our user then we need to update for every user
if self.is_mine_id(user_id):
# First, if the joining user is our local user then we need an
# update for every other user in the room.
if self.is_mine_id(joining_user_id):
for other_user_id in other_users_in_room:
to_insert.add((user_id, other_user_id))
updates_to_users_who_share_rooms.add(
(joining_user_id, other_user_id)
)
# Next we need to update for every local user in the room
# Next, we need an update for every other local user in the room
# that they now share a room with the joining user.
for other_user_id in other_users_in_room:
if self.is_mine_id(other_user_id):
to_insert.add((other_user_id, user_id))
updates_to_users_who_share_rooms.add(
(other_user_id, joining_user_id)
)
if to_insert:
await self.store.add_users_who_share_private_room(room_id, to_insert)
if updates_to_users_who_share_rooms:
await self.store.add_users_who_share_private_room(
room_id, updates_to_users_who_share_rooms
)
async def _handle_remove_user(self, room_id: str, user_id: str) -> None:
"""Called when when someone leaves a room. The user may be local or remote.