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

Add support for MSC4115 #17104

Merged
merged 9 commits into from
Apr 29, 2024
Merged
Show file tree
Hide file tree
Changes from 6 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/17104.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add support for MSC4115 (membership metadata on events).
4 changes: 2 additions & 2 deletions docker/complement/conf/workers-shared-extra.yaml.j2
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,6 @@ allow_device_name_lookup_over_federation: true
## Experimental Features ##

experimental_features:
# client-side support for partial state in /send_join responses
faster_joins: true
# Enable support for polls
msc3381_polls_enabled: true
# Enable deleting device-specific notification settings stored in account data
Expand All @@ -105,6 +103,8 @@ experimental_features:
# no UIA for x-signing upload for the first time
msc3967_enabled: true

msc4115_membership_on_events: true

server_notices:
system_mxid_localpart: _server
system_mxid_display_name: "Server Alert"
Expand Down
7 changes: 7 additions & 0 deletions synapse/api/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,13 @@ class EventContentFields:
TO_DEVICE_MSGID: Final = "org.matrix.msgid"


class EventUnsignedContentFields:
"""Fields found inside the 'unsigned' data on events"""

# Requesting user's membership, per MSC4115
MSC4115_MEMBERSHIP: Final = "io.element.msc4115.membership"


class RoomTypes:
"""Understood values of the room_type field of m.room.create events."""

Expand Down
4 changes: 4 additions & 0 deletions synapse/config/experimental.py
Original file line number Diff line number Diff line change
Expand Up @@ -422,3 +422,7 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None:
"MSC4108 requires MSC3861 to be enabled",
("experimental", "msc4108_delegation_endpoint"),
)

self.msc4115_membership_on_events = experimental.get(
"msc4115_membership_on_events", False
)
23 changes: 20 additions & 3 deletions synapse/events/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
from synapse.api.room_versions import RoomVersion
from synapse.types import JsonDict, Requester

from . import EventBase
from . import EventBase, make_event_from_dict

if TYPE_CHECKING:
from synapse.handlers.relations import BundledAggregations
Expand Down Expand Up @@ -82,8 +82,6 @@ def prune_event(event: EventBase) -> EventBase:
"""
pruned_event_dict = prune_event_dict(event.room_version, event.get_dict())

from . import make_event_from_dict

pruned_event = make_event_from_dict(
pruned_event_dict, event.room_version, event.internal_metadata.get_dict()
)
Expand All @@ -101,6 +99,25 @@ def prune_event(event: EventBase) -> EventBase:
return pruned_event


def clone_event(event: EventBase) -> EventBase:
"""Take a copy of the event.

This is mostly useful because it does a *shallow* copy of the `unsigned` data,
which means it can then be updated without corrupting the in-memory cache.
Copy link
Member

Choose a reason for hiding this comment

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

Would you not want a deep copy of the unsigned dictionary instead, such that you can update it without affecting the original?

Granted, your unit test proves that this does the right thing!

Copy link
Member Author

Choose a reason for hiding this comment

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

Well. As it happens, we're updating a top-level key in the unsigned dict, so a shallow copy is enough.

I mean doing a shallow copy isn't deliberate on my part, it's just what calling make_event_from_dict(event.get_dict(), ...) happens to do.

Generally this whole thing seems like a big mess, and I kinda hate it.

  • Why do both event.get_dict and make_event_from_dict copy unsigned, when they doesn't copy any of the other properties (like, say, content)?
  • If they are going to copy it, why is it a shallow copy?
  • Why are stream_ordering and outlier part of internal_metadata in the in-memory model when they are separate in the database, so need magic handling?
  • Also, why is the value of unsigned not protected by use_frozen_dicts like the rest of the event, so that I nearly shot my own foot off here?

Nobody knows, but fixing these things aren't really today jobs.

I'll add some more comments which might help here.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the thorough explanation. I agree that the state of those functions are a mess, and look like the result of organic growth. They could use a refactor.

Agreed that that's a problem for another day.

"""
new_event = make_event_from_dict(
event.get_dict(), event.room_version, event.internal_metadata.get_dict()
)

# copy the internal fields
new_event.internal_metadata.stream_ordering = (
event.internal_metadata.stream_ordering
)
new_event.internal_metadata.outlier = event.internal_metadata.outlier

return new_event


def prune_event_dict(room_version: RoomVersion, event_dict: JsonDict) -> JsonDict:
"""Redacts the event_dict in the same way as `prune_event`, except it
operates on dicts rather than event objects
Expand Down
6 changes: 5 additions & 1 deletion synapse/handlers/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ def __init__(self, hs: "HomeServer"):
self._device_handler = hs.get_device_handler()
self._storage_controllers = hs.get_storage_controllers()
self._state_storage_controller = self._storage_controllers.state
self._hs_config = hs.config
self._msc3866_enabled = hs.config.experimental.msc3866.enabled

async def get_whois(self, user: UserID) -> JsonMapping:
Expand Down Expand Up @@ -217,7 +218,10 @@ async def export_user_data(self, user_id: str, writer: "ExfiltrationWriter") ->
)

events = await filter_events_for_client(
self._storage_controllers, user_id, events
self._storage_controllers,
user_id,
events,
msc4115_membership_on_events=self._hs_config.experimental.msc4115_membership_on_events,
)

writer.write_events(room_id, events)
Expand Down
7 changes: 6 additions & 1 deletion synapse/handlers/events.py
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ class EventHandler:
def __init__(self, hs: "HomeServer"):
self.store = hs.get_datastores().main
self._storage_controllers = hs.get_storage_controllers()
self._config = hs.config

async def get_event(
self,
Expand Down Expand Up @@ -189,7 +190,11 @@ async def get_event(
is_peeking = not is_user_in_room

filtered = await filter_events_for_client(
self._storage_controllers, user.to_string(), [event], is_peeking=is_peeking
self._storage_controllers,
user.to_string(),
[event],
is_peeking=is_peeking,
msc4115_membership_on_events=self._config.experimental.msc4115_membership_on_events,
)

if not filtered:
Expand Down
7 changes: 6 additions & 1 deletion synapse/handlers/initial_sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,10 @@ async def handle_room(event: RoomsForUser) -> None:
).addErrback(unwrapFirstError)

messages = await filter_events_for_client(
self._storage_controllers, user_id, messages
self._storage_controllers,
user_id,
messages,
msc4115_membership_on_events=self.hs.config.experimental.msc4115_membership_on_events,
)

start_token = now_token.copy_and_replace(StreamKeyType.ROOM, token)
Expand Down Expand Up @@ -380,6 +383,7 @@ async def _room_initial_sync_parted(
requester.user.to_string(),
messages,
is_peeking=is_peeking,
msc4115_membership_on_events=self.hs.config.experimental.msc4115_membership_on_events,
)

start_token = StreamToken.START.copy_and_replace(StreamKeyType.ROOM, token)
Expand Down Expand Up @@ -494,6 +498,7 @@ async def get_receipts() -> List[JsonMapping]:
requester.user.to_string(),
messages,
is_peeking=is_peeking,
msc4115_membership_on_events=self.hs.config.experimental.msc4115_membership_on_events,
)

start_token = now_token.copy_and_replace(StreamKeyType.ROOM, token)
Expand Down
1 change: 1 addition & 0 deletions synapse/handlers/pagination.py
Original file line number Diff line number Diff line change
Expand Up @@ -623,6 +623,7 @@ async def get_messages(
user_id,
events,
is_peeking=(member_event_id is None),
msc4115_membership_on_events=self.hs.config.experimental.msc4115_membership_on_events,
)

# if after the filter applied there are no more events
Expand Down
3 changes: 3 additions & 0 deletions synapse/handlers/relations.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ def __init__(self, hs: "HomeServer"):
self._event_handler = hs.get_event_handler()
self._event_serializer = hs.get_event_client_serializer()
self._event_creation_handler = hs.get_event_creation_handler()
self._config = hs.config

async def get_relations(
self,
Expand Down Expand Up @@ -163,6 +164,7 @@ async def get_relations(
user_id,
events,
is_peeking=(member_event_id is None),
msc4115_membership_on_events=self._config.experimental.msc4115_membership_on_events,
)

# The relations returned for the requested event do include their
Expand Down Expand Up @@ -608,6 +610,7 @@ async def get_threads(
user_id,
events,
is_peeking=(member_event_id is None),
msc4115_membership_on_events=self._config.experimental.msc4115_membership_on_events,
)

aggregations = await self.get_bundled_aggregations(
Expand Down
1 change: 1 addition & 0 deletions synapse/handlers/room.py
Original file line number Diff line number Diff line change
Expand Up @@ -1476,6 +1476,7 @@ async def filter_evts(events: List[EventBase]) -> List[EventBase]:
user.to_string(),
events,
is_peeking=is_peeking,
msc4115_membership_on_events=self.hs.config.experimental.msc4115_membership_on_events,
)

event = await self.store.get_event(
Expand Down
20 changes: 16 additions & 4 deletions synapse/handlers/search.py
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,10 @@ async def _search_by_rank(
filtered_events = await search_filter.filter([r["event"] for r in results])

events = await filter_events_for_client(
self._storage_controllers, user.to_string(), filtered_events
self._storage_controllers,
user.to_string(),
filtered_events,
msc4115_membership_on_events=self.hs.config.experimental.msc4115_membership_on_events,
)

events.sort(key=lambda e: -rank_map[e.event_id])
Expand Down Expand Up @@ -579,7 +582,10 @@ async def _search_by_recent(
filtered_events = await search_filter.filter([r["event"] for r in results])

events = await filter_events_for_client(
self._storage_controllers, user.to_string(), filtered_events
self._storage_controllers,
user.to_string(),
filtered_events,
msc4115_membership_on_events=self.hs.config.experimental.msc4115_membership_on_events,
)

room_events.extend(events)
Expand Down Expand Up @@ -664,11 +670,17 @@ async def _calculate_event_contexts(
)

events_before = await filter_events_for_client(
self._storage_controllers, user.to_string(), res.events_before
self._storage_controllers,
user.to_string(),
res.events_before,
msc4115_membership_on_events=self.hs.config.experimental.msc4115_membership_on_events,
)

events_after = await filter_events_for_client(
self._storage_controllers, user.to_string(), res.events_after
self._storage_controllers,
user.to_string(),
res.events_after,
msc4115_membership_on_events=self.hs.config.experimental.msc4115_membership_on_events,
)

context: JsonDict = {
Expand Down
2 changes: 2 additions & 0 deletions synapse/handlers/sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -596,6 +596,7 @@ async def _load_filtered_recents(
sync_config.user.to_string(),
recents,
always_include_ids=current_state_ids,
msc4115_membership_on_events=self.hs_config.experimental.msc4115_membership_on_events,
)
log_kv({"recents_after_visibility_filtering": len(recents)})
else:
Expand Down Expand Up @@ -681,6 +682,7 @@ async def _load_filtered_recents(
sync_config.user.to_string(),
loaded_recents,
always_include_ids=current_state_ids,
msc4115_membership_on_events=self.hs_config.experimental.msc4115_membership_on_events,
)

loaded_recents = []
Expand Down
1 change: 1 addition & 0 deletions synapse/notifier.py
Original file line number Diff line number Diff line change
Expand Up @@ -721,6 +721,7 @@ async def check_for_updates(
user.to_string(),
new_events,
is_peeking=is_peeking,
msc4115_membership_on_events=self.hs.config.experimental.msc4115_membership_on_events,
)
elif keyname == StreamKeyType.PRESENCE:
now = self.clock.time_msec()
Expand Down
5 changes: 4 additions & 1 deletion synapse/push/mailer.py
Original file line number Diff line number Diff line change
Expand Up @@ -513,7 +513,10 @@ async def _get_notif_vars(
}

the_events = await filter_events_for_client(
self._storage_controllers, user_id, results.events_before
self._storage_controllers,
user_id,
results.events_before,
msc4115_membership_on_events=self.hs.config.experimental.msc4115_membership_on_events,
)
the_events.append(notif_event)

Expand Down
Loading
Loading