Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move call invite filtering logic to filter_events_for_client #17782

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions changelog.d/17782.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Improve event filtering for Simplified Sliding Sync.
2 changes: 0 additions & 2 deletions synapse/handlers/sliding_sync/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -673,8 +673,6 @@ async def get_room_sync_data(
!= Membership.JOIN,
filter_send_to_client=True,
)
# TODO: Filter out `EventTypes.CallInvite` in public rooms,
# see https:/element-hq/synapse/issues/17359

# TODO: Handle timeline gaps (`get_timeline_gaps()`)

Expand Down
12 changes: 1 addition & 11 deletions synapse/handlers/sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@
Direction,
EventContentFields,
EventTypes,
JoinRules,
Membership,
)
from synapse.api.filtering import FilterCollection
Expand Down Expand Up @@ -948,22 +947,13 @@ async def _load_filtered_recents(
)
)

filtered_recents = await filter_events_for_client(
loaded_recents = await filter_events_for_client(
self._storage_controllers,
sync_config.user.to_string(),
loaded_recents,
always_include_ids=current_state_ids,
)

loaded_recents = []
for event in filtered_recents:
if event.type == EventTypes.CallInvite:
room_info = await self.store.get_room_with_stats(event.room_id)
assert room_info is not None
if room_info.join_rules == JoinRules.PUBLIC:
continue
loaded_recents.append(event)

log_kv({"loaded_recents_after_client_filtering": len(loaded_recents)})

loaded_recents.extend(recents)
Expand Down
13 changes: 12 additions & 1 deletion synapse/visibility.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
EventTypes,
EventUnsignedContentFields,
HistoryVisibility,
JoinRules,
Membership,
)
from synapse.events import EventBase
Expand Down Expand Up @@ -117,7 +118,7 @@ async def filter_events_for_client(
[event.event_id for event in events],
)

types = (_HISTORY_VIS_KEY, (EventTypes.Member, user_id))
types = (_HISTORY_VIS_KEY, (EventTypes.Member, user_id), (EventTypes.JoinRules, ""))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

filter_events_for_client(...) is a pretty slow operation already and a hot path for so many endpoints.

It would be good to conditionally request EventTypes.JoinRules only if we see an event of type EventTypes.CallInvite.


# we exclude outliers at this point, and then handle them separately later
event_id_to_state = await storage.state.get_state_for_events(
Expand Down Expand Up @@ -156,6 +157,16 @@ def allowed(event: EventBase) -> Optional[EventBase]:
if filtered is None:
return None

# Filter out call invites in public rooms, as this would potentially
# ring a lot of users.
if state_after_event is not None and event.type == EventTypes.CallInvite:
room_join_rules = state_after_event.get((EventTypes.JoinRules, ""))
if (
room_join_rules is not None
and room_join_rules.content.get("join_rule") == JoinRules.PUBLIC
):
return None
MadLittleMods marked this conversation as resolved.
Show resolved Hide resolved

# Annotate the event with the user's membership after the event.
#
# Normally we just look in `state_after_event`, but if the event is an outlier
Expand Down
6 changes: 3 additions & 3 deletions tests/rest/client/test_relations.py
Original file line number Diff line number Diff line change
Expand Up @@ -1181,7 +1181,7 @@ def assert_annotations(bundled_aggregations: JsonDict) -> None:
bundled_aggregations,
)

self._test_bundled_aggregations(RelationTypes.REFERENCE, assert_annotations, 6)
self._test_bundled_aggregations(RelationTypes.REFERENCE, assert_annotations, 7)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To explain this change; the extra database call is because (EventTypes.JoinRules, "") was added to the types we query for in filter_events_for_client(...)


def test_thread(self) -> None:
"""
Expand Down Expand Up @@ -1226,7 +1226,7 @@ def assert_thread(bundled_aggregations: JsonDict) -> None:

# The "user" sent the root event and is making queries for the bundled
# aggregations: they have participated.
self._test_bundled_aggregations(RelationTypes.THREAD, _gen_assert(True), 6)
self._test_bundled_aggregations(RelationTypes.THREAD, _gen_assert(True), 7)
# The "user2" sent replies in the thread and is making queries for the
# bundled aggregations: they have participated.
#
Expand Down Expand Up @@ -1287,7 +1287,7 @@ def assert_thread(bundled_aggregations: JsonDict) -> None:
bundled_aggregations["latest_event"].get("unsigned"),
)

self._test_bundled_aggregations(RelationTypes.THREAD, assert_thread, 6)
self._test_bundled_aggregations(RelationTypes.THREAD, assert_thread, 7)

def test_nested_thread(self) -> None:
"""
Expand Down
Loading