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

Check appservice user interest against the local users instead of all users (get_users_in_room mis-use) #13958

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
99a623d
Check appservice user interest against the local users instead of all…
MadLittleMods Sep 29, 2022
806b255
Remove non-appservice usages split out to other PRs
MadLittleMods Sep 29, 2022
5f0f815
Add changelog
MadLittleMods Sep 29, 2022
a8be41b
Revert back to using our own `_matches_user_in_member_list` thing
MadLittleMods Sep 29, 2022
72985df
Rename variables to 'local' to be obvious our intention
MadLittleMods Sep 29, 2022
92b9da2
Fix tests
MadLittleMods Sep 30, 2022
5d3c6a3
Wrapping happens at 88 chars
MadLittleMods Sep 30, 2022
76435c7
Add actual homeserver tests for local/remote interesting to appservic…
MadLittleMods Sep 30, 2022
4451998
Clarify interested/control and lints
MadLittleMods Sep 30, 2022
1218f03
Revert mock
MadLittleMods Sep 30, 2022
7bd3803
Add test descriptions
MadLittleMods Sep 30, 2022
33f718c
Revert "Clarify interested/control and lints"
MadLittleMods Sep 30, 2022
cf8299b
Revert "Add test descriptions"
MadLittleMods Sep 30, 2022
3de90e6
Revert "Revert mock"
MadLittleMods Sep 30, 2022
ab33cd6
Revert "Add actual homeserver tests for local/remote interesting to a…
MadLittleMods Sep 30, 2022
3223512
Move tests over from #14000
MadLittleMods Oct 4, 2022
d913ceb
Merge branch 'develop' into madlittlemods/13942-appservice-get_users_…
MadLittleMods Oct 4, 2022
4f29e75
Merge branch 'develop' into madlittlemods/13942-appservice-get_users_…
MadLittleMods Oct 24, 2022
2665aa0
Update changelog
MadLittleMods Oct 24, 2022
39e2ead
Add specific test to make sure local interesting user events are pick…
MadLittleMods Oct 25, 2022
33a5b70
Update upgrade notes
MadLittleMods Oct 25, 2022
92400ff
move comma
anoadragon453 Oct 27, 2022
426ef5c
Merge branch 'develop' of github.com:matrix-org/synapse into madlittl…
anoadragon453 Oct 27, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 3 additions & 7 deletions synapse/appservice/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -172,15 +172,11 @@ async def _matches_user_in_member_list(
Returns:
True if this service would like to know about this room.
"""
member_list = await store.get_users_in_room(
room_id, on_invalidate=cache_context.invalidate
app_service_users_in_room = await store.get_app_service_users_in_room(
room_id, self, on_invalidate=cache_context.invalidate
)

# check joined member events
for user_id in member_list:
if self.is_interested_in_user(user_id):
return True
return False
return len(app_service_users_in_room) > 0
MadLittleMods marked this conversation as resolved.
Show resolved Hide resolved

def is_interested_in_user(
self,
Expand Down
2 changes: 1 addition & 1 deletion synapse/handlers/room_member.py
Original file line number Diff line number Diff line change
Expand Up @@ -1150,7 +1150,7 @@ async def transfer_room_state_on_room_upgrade(
logger.info("Transferring room state from %s to %s", old_room_id, room_id)

# Find all local users that were in the old room and copy over each user's state
users = await self.store.get_users_in_room(old_room_id)
users = await self.store.get_local_users_in_room(old_room_id)
MadLittleMods marked this conversation as resolved.
Show resolved Hide resolved
await self.copy_user_state_on_room_upgrade(old_room_id, room_id, users)

# Add new room to the room directory if the old room was there
Expand Down
1 change: 1 addition & 0 deletions synapse/handlers/user_directory.py
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,7 @@ async def _track_user_joined_room(self, room_id: str, user_id: str) -> None:
if is_public:
await self.store.add_users_in_public_rooms(room_id, (user_id,))
else:
# TODO: get_local_users_in_room here
MadLittleMods marked this conversation as resolved.
Show resolved Hide resolved
users_in_room = await self.store.get_users_in_room(room_id)
other_users_in_room = [
other
Expand Down
14 changes: 13 additions & 1 deletion synapse/storage/databases/main/appservice.py
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,19 @@ async def get_app_service_users_in_room(
app_service: "ApplicationService",
cache_context: _CacheContext,
) -> List[str]:
users_in_room = await self.get_users_in_room(
"""
Get all users in a room that the appservice controls.

Args:
room_id: The room to check in.
app_service: The application service to check interest/control against

Returns:
List of user IDs that the appservice controls.
"""
# We can use `get_local_users_in_room(...)` here because an application
# service can only act on behalf of users of the server it's on.
users_in_room = await self.get_local_users_in_room(
room_id, on_invalidate=cache_context.invalidate
)
return list(filter(app_service.is_interested_in_user, users_in_room))
Expand Down
19 changes: 12 additions & 7 deletions synapse/storage/databases/main/roommember.py
Original file line number Diff line number Diff line change
Expand Up @@ -152,13 +152,18 @@ async def get_users_in_room(self, room_id: str) -> List[str]:
`get_current_hosts_in_room()` and so we can re-use the cache but it's
not horrible to have here either.

Uses `m.room.member`s in the room state at the current forward extremities to
determine which users are in the room.

Will return inaccurate results for rooms with partial state, since the state for
the forward extremities of those rooms will exclude most members. We may also
calculate room state incorrectly for such rooms and believe that a member is or
is not in the room when the opposite is true.
Uses `m.room.member`s in the room state at the current forward
extremities to determine which users are in the room.

Will return inaccurate results for rooms with partial state, since the
state for the forward extremities of those rooms will exclude most
members. We may also calculate room state incorrectly for such rooms and
believe that a member is or is not in the room when the opposite is
true.

Note: If you only care about users in the room local to the homeserver,
use `get_local_users_in_room(...)` instead which will be more
performant.
MadLittleMods marked this conversation as resolved.
Show resolved Hide resolved
"""
return await self.db_pool.runInteraction(
"get_users_in_room", self.get_users_in_room_txn, room_id
Expand Down