From a6c349e57cff47f5da9c32dbdab624ae6dac2500 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 22 Jun 2026 13:52:05 +0100 Subject: [PATCH] Add create_config helper Rather than implicitly updating the config in `serialize_event`, let's add a helper for creating the config that does the necessary checks/updates itself. --- rust/src/events/serialize.rs | 27 -------- synapse/appservice/api.py | 4 +- synapse/events/utils.py | 97 ++++++++++++++++++---------- synapse/handlers/events.py | 4 +- synapse/handlers/initial_sync.py | 14 ++-- synapse/handlers/message.py | 3 +- synapse/handlers/relations.py | 6 +- synapse/handlers/search.py | 6 +- synapse/rest/admin/events.py | 3 +- synapse/rest/admin/rooms.py | 2 +- synapse/rest/client/events.py | 3 +- synapse/rest/client/notifications.py | 3 +- synapse/rest/client/room.py | 10 +-- synapse/rest/client/sync.py | 6 +- synapse/synapse_rust/events.pyi | 19 ++---- tests/events/test_utils.py | 49 +++++--------- 16 files changed, 120 insertions(+), 136 deletions(-) diff --git a/rust/src/events/serialize.rs b/rust/src/events/serialize.rs index 1a608655fe..21af7635be 100644 --- a/rust/src/events/serialize.rs +++ b/rust/src/events/serialize.rs @@ -130,16 +130,6 @@ pub struct SerializeEventConfig { impl SerializeEventConfig { #[new] #[allow(clippy::too_many_arguments)] - #[pyo3(signature = ( - *, - as_client_event = true, - event_format = EventFormat::ClientV1, - requester = None, - event_field_allowlist = None, - include_stripped_room_state = false, - include_admin_metadata = false, - msc4354_enabled = false, - ))] fn new( as_client_event: bool, event_format: EventFormat, @@ -199,23 +189,6 @@ impl SerializeEventConfig { fn msc4354_enabled(&self) -> bool { self.msc4354_enabled } - - /// Returns a copy of this config with `include_admin_metadata` enabled. - fn for_admin(&self) -> Self { - Self { - include_admin_metadata: true, - ..self.clone() - } - } - - /// Returns a copy of this config with `msc4354_enabled` set as given. - #[pyo3(signature = (enabled = true))] - fn with_msc4354(&self, enabled: bool) -> Self { - Self { - msc4354_enabled: enabled, - ..self.clone() - } - } } /// Synchronously serialize a batch of events for clients. diff --git a/synapse/appservice/api.py b/synapse/appservice/api.py index 66c962e17d..cae0101d52 100644 --- a/synapse/appservice/api.py +++ b/synapse/appservice/api.py @@ -40,7 +40,7 @@ from synapse.appservice import ( TransactionUnusedFallbackKeys, ) from synapse.events import EventBase -from synapse.events.utils import FilteredEvent, SerializeEventConfig +from synapse.events.utils import FilteredEvent from synapse.http.client import SimpleHttpClient, is_unknown_endpoint from synapse.logging import opentracing from synapse.metrics import SERVER_NAME_LABEL @@ -547,7 +547,7 @@ class ApplicationServiceApi(SimpleHttpClient): return await self._event_serializer.serialize_events( [FilteredEvent(event=e, membership=None) for e in events], time_now, - config=SerializeEventConfig( + config=await self._event_serializer.create_config( as_client_event=True, # If this is an invite or a knock membership event, then include # any stripped state alongside the event. We could narrow this diff --git a/synapse/events/utils.py b/synapse/events/utils.py index 081a8b5bdd..a396ad58b4 100644 --- a/synapse/events/utils.py +++ b/synapse/events/utils.py @@ -48,6 +48,7 @@ from synapse.synapse_rust.events import ( redact_event, serialize_events, ) +from synapse.synapse_rust.types import Requester from synapse.types import JsonDict from . import EventBase, StrippedStateEvent @@ -135,15 +136,6 @@ class FilteredEvent: return cls(event=event, membership=None) -_DEFAULT_SERIALIZE_EVENT_CONFIG = SerializeEventConfig() - - -def make_config_for_admin(existing: SerializeEventConfig) -> SerializeEventConfig: - # Thin wrapper kept for callers; see SerializeEventConfig.for_admin for the - # admin-only options it sets. - return existing.for_admin() - - class EventClientSerializer: """Serializes events that are to be sent to clients. @@ -160,12 +152,65 @@ class EventClientSerializer: ADD_EXTRA_FIELDS_TO_UNSIGNED_CLIENT_EVENT_CALLBACK ] = [] + async def create_config( + self, + *, + as_client_event: bool = True, + event_format: EventFormat = EventFormat.ClientV1, + requester: Requester | None = None, + event_field_allowlist: list[str] | None = None, + include_stripped_room_state: bool = False, + include_admin_metadata: bool | None = None, + ) -> SerializeEventConfig: + """ + Create a new SerializeEventConfig for the given parameters. + + Helper method that sets the `include_admin_metadata` field based on + whether the requester is a server admin if it is not explicitly + provided. Also sets the `msc4354_enabled` field based on the homeserver + config. + + Args: + as_client_event: Whether to serialize the events as client events. + event_format: The format to serialize events in. requester: The user + requesting the events, if any. Used to determine + whether to include admin-only metadata in the serialized events. + event_field_allowlist: A list of event fields to include in the + serialized events. + include_stripped_room_state: Whether to include stripped room state + in the serialized events. + include_admin_metadata: Whether to include admin-only metadata in + the serialized events. If None, this will be determined based on + whether the requester is a server admin. + Returns: + A SerializeEventConfig instance. + """ + + # If include_admin_metadata is None, determine whether to include + # admin-only metadata based on the requester. + if include_admin_metadata is None: + # Check if the requester is a server admin. + if requester is not None and await self._auth.is_server_admin(requester): + include_admin_metadata = True + else: + include_admin_metadata = False + + return SerializeEventConfig( + as_client_event=as_client_event, + event_format=event_format, + requester=requester, + event_field_allowlist=event_field_allowlist, + include_stripped_room_state=include_stripped_room_state, + include_admin_metadata=include_admin_metadata, + msc4354_enabled=self._config.experimental.msc4354_enabled, + ) + async def serialize_event( self, event: JsonDict | FilteredEvent, time_now: int, *, - config: SerializeEventConfig = _DEFAULT_SERIALIZE_EVENT_CONFIG, + config: SerializeEventConfig | None = None, bundle_aggregations: dict[str, "BundledAggregations"] | None = None, redaction_map: Mapping[str, "EventBase"] | None = None, ) -> JsonDict: @@ -187,7 +232,9 @@ class EventClientSerializer: if not isinstance(event, FilteredEvent): return event - config = await self._update_config(config) + if config is None: + # Generate default config if none was provided. + config = await self.create_config() # Perform all the async DB/IO work up front, then run the synchronous # serialization core. @@ -204,28 +251,6 @@ class EventClientSerializer: unsigned_additions=unsigned_additions, )[0] - async def _update_config( - self, config: SerializeEventConfig - ) -> SerializeEventConfig: - """Update the config based on the requester and server config.""" - - # Force-enable server admin metadata because the only time an event with - # relevant metadata will be when the admin requested it via their admin - # client config account data. Also, it's "just" some `unsigned` fields, so - # shouldn't cause much in terms of problems to downstream consumers. - # - # The requester is constant across the whole (recursive) serialization, - # so we only need to resolve this once. - if config.requester is not None and await self._auth.is_server_admin( - config.requester - ): - config = make_config_for_admin(config) - - if self._config.experimental.msc4354_enabled: - config = config.with_msc4354(True) - - return config - async def _prepare_serialization( self, events: Collection[FilteredEvent], @@ -313,7 +338,7 @@ class EventClientSerializer: events: Collection[JsonDict | FilteredEvent], time_now: int, *, - config: SerializeEventConfig = _DEFAULT_SERIALIZE_EVENT_CONFIG, + config: SerializeEventConfig | None = None, bundle_aggregations: dict[str, "BundledAggregations"] | None = None, ) -> list[JsonDict]: """Serializes multiple events. @@ -334,7 +359,9 @@ class EventClientSerializer: str(len(events)), ) - config = await self._update_config(config) + if config is None: + # Generate default config if none was provided. + config = await self.create_config() filtered_events = [e for e in events if isinstance(e, FilteredEvent)] diff --git a/synapse/handlers/events.py b/synapse/handlers/events.py index 2518716bc7..56ded1634e 100644 --- a/synapse/handlers/events.py +++ b/synapse/handlers/events.py @@ -25,7 +25,7 @@ from typing import TYPE_CHECKING, Iterable from synapse.api.constants import EduTypes, EventTypes, Membership, PresenceState from synapse.api.errors import AuthError, SynapseError -from synapse.events.utils import FilteredEvent, SerializeEventConfig +from synapse.events.utils import FilteredEvent from synapse.handlers.presence import format_user_presence_state from synapse.storage.databases.main.events_worker import EventRedactBehaviour from synapse.streams.config import PaginationConfig @@ -129,7 +129,7 @@ class EventStreamHandler: chunks = await self._event_serializer.serialize_events( events, time_now, - config=SerializeEventConfig( + config=await self._event_serializer.create_config( as_client_event=as_client_event, requester=requester ), ) diff --git a/synapse/handlers/initial_sync.py b/synapse/handlers/initial_sync.py index 591a0aefd3..56f4d86d41 100644 --- a/synapse/handlers/initial_sync.py +++ b/synapse/handlers/initial_sync.py @@ -30,7 +30,7 @@ from synapse.api.constants import ( Membership, ) from synapse.api.errors import SynapseError -from synapse.events.utils import FilteredEvent, SerializeEventConfig +from synapse.events.utils import FilteredEvent from synapse.events.validator import EventValidator from synapse.handlers.presence import format_user_presence_state from synapse.handlers.receipts import ReceiptEventSource @@ -169,7 +169,9 @@ class InitialSyncHandler: public_room_ids = await self.store.get_public_room_ids() - serializer_options = SerializeEventConfig(as_client_event=as_client_event) + serializer_options = await self._event_serializer.create_config( + as_client_event=as_client_event + ) async def handle_room(event: RoomsForUser) -> None: d: JsonDict = { @@ -395,7 +397,9 @@ class InitialSyncHandler: end_token = StreamToken.START.copy_and_replace(StreamKeyType.ROOM, stream_token) time_now = self.clock.time_msec() - serialize_options = SerializeEventConfig(requester=requester) + serialize_options = await self._event_serializer.create_config( + requester=requester + ) return { "membership": membership, @@ -436,7 +440,9 @@ class InitialSyncHandler: # TODO: These concurrently time_now = self.clock.time_msec() - serialize_options = SerializeEventConfig(requester=requester) + serialize_options = await self._event_serializer.create_config( + requester=requester + ) # Don't bundle aggregations as this is a deprecated API. state = await self._event_serializer.serialize_events( [FilteredEvent.state(e) for e in current_state.values()], diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index ed04547d42..9785e8efc0 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -64,7 +64,6 @@ from synapse.events.snapshot import ( ) from synapse.events.utils import ( FilteredEvent, - SerializeEventConfig, maybe_upsert_event_field, ) from synapse.events.validator import EventValidator @@ -269,7 +268,7 @@ class MessageHandler: events = await self._event_serializer.serialize_events( [FilteredEvent.state(e) for e in room_state.values()], self.clock.time_msec(), - config=SerializeEventConfig(requester=requester), + config=await self._event_serializer.create_config(requester=requester), ) return events diff --git a/synapse/handlers/relations.py b/synapse/handlers/relations.py index 4fd301a18e..a8db082feb 100644 --- a/synapse/handlers/relations.py +++ b/synapse/handlers/relations.py @@ -31,7 +31,7 @@ from typing import ( from synapse.api.constants import Direction, EventTypes, RelationTypes from synapse.api.errors import SynapseError from synapse.events import EventBase, relation_from_event -from synapse.events.utils import FilteredEvent, SerializeEventConfig +from synapse.events.utils import FilteredEvent from synapse.logging.context import make_deferred_yieldable, run_in_background from synapse.logging.opentracing import trace from synapse.storage.databases.main.relations import ThreadsNextBatch, _RelatedEvent @@ -150,7 +150,9 @@ class RelationsHandler: ) now = self._clock.time_msec() - serialize_options = SerializeEventConfig(requester=requester) + serialize_options = await self._event_serializer.create_config( + requester=requester + ) return_value: JsonDict = { "chunk": await self._event_serializer.serialize_events( filtered_events, diff --git a/synapse/handlers/search.py b/synapse/handlers/search.py index 30e072d011..eb0492ff59 100644 --- a/synapse/handlers/search.py +++ b/synapse/handlers/search.py @@ -29,7 +29,7 @@ from unpaddedbase64 import decode_base64, encode_base64 from synapse.api.constants import EventTypes, Membership from synapse.api.errors import NotFoundError, SynapseError from synapse.api.filtering import Filter -from synapse.events.utils import FilteredEvent, SerializeEventConfig +from synapse.events.utils import FilteredEvent from synapse.types import JsonDict, Requester, StrCollection, StreamKeyType, UserID from synapse.types.state import StateFilter from synapse.visibility import filter_and_transform_events_for_client @@ -377,7 +377,9 @@ class SearchHandler: # blocking calls after this. Otherwise, the 'age' will be wrong. time_now = self.clock.time_msec() - serialize_options = SerializeEventConfig(requester=requester) + serialize_options = await self._event_serializer.create_config( + requester=requester + ) for context in contexts.values(): context["events_before"] = await self._event_serializer.serialize_events( diff --git a/synapse/rest/admin/events.py b/synapse/rest/admin/events.py index 284bc9a47c..7dbd7f5d2b 100644 --- a/synapse/rest/admin/events.py +++ b/synapse/rest/admin/events.py @@ -5,7 +5,6 @@ from synapse.api.errors import NotFoundError from synapse.events.utils import ( EventFormat, FilteredEvent, - SerializeEventConfig, ) from synapse.http.servlet import RestServlet from synapse.http.site import SynapseRequest @@ -57,7 +56,7 @@ class EventRestServlet(RestServlet): if event is None: raise NotFoundError("Event not found") - config = SerializeEventConfig( + config = await self._event_serializer.create_config( as_client_event=False, event_format=EventFormat.Raw, requester=requester, diff --git a/synapse/rest/admin/rooms.py b/synapse/rest/admin/rooms.py index f6693e0923..e47b6e9efe 100644 --- a/synapse/rest/admin/rooms.py +++ b/synapse/rest/admin/rooms.py @@ -1028,7 +1028,7 @@ class RoomMessagesRestServlet(RestServlet): ): as_client_event = False - serialize_options = SerializeEventConfig( + serialize_options = await self._event_serializer.create_config( as_client_event=as_client_event, requester=requester ) diff --git a/synapse/rest/client/events.py b/synapse/rest/client/events.py index de73c96fd0..f5b894038e 100644 --- a/synapse/rest/client/events.py +++ b/synapse/rest/client/events.py @@ -25,7 +25,6 @@ import logging from typing import TYPE_CHECKING from synapse.api.errors import SynapseError -from synapse.events.utils import SerializeEventConfig from synapse.http.server import HttpServer from synapse.http.servlet import RestServlet, parse_string from synapse.http.site import SynapseRequest @@ -104,7 +103,7 @@ class EventRestServlet(RestServlet): result = await self._event_serializer.serialize_event( event, self.clock.time_msec(), - config=SerializeEventConfig(requester=requester), + config=await self._event_serializer.create_config(requester=requester), ) return 200, result else: diff --git a/synapse/rest/client/notifications.py b/synapse/rest/client/notifications.py index d56b7cf3e5..ae3893d296 100644 --- a/synapse/rest/client/notifications.py +++ b/synapse/rest/client/notifications.py @@ -26,7 +26,6 @@ from synapse.api.constants import ReceiptTypes from synapse.events.utils import ( EventFormat, FilteredEvent, - SerializeEventConfig, ) from synapse.http.server import HttpServer from synapse.http.servlet import RestServlet, parse_integer, parse_string @@ -98,7 +97,7 @@ class NotificationsServlet(RestServlet): next_token = None - serialize_options = SerializeEventConfig( + serialize_options = await self._event_serializer.create_config( event_format=EventFormat.ClientV2WithoutRoomId, requester=requester, ) diff --git a/synapse/rest/client/room.py b/synapse/rest/client/room.py index 29ca67babd..36f638e236 100644 --- a/synapse/rest/client/room.py +++ b/synapse/rest/client/room.py @@ -289,7 +289,7 @@ class RoomStateEventRestServlet(RestServlet): event = await self._event_serializer.serialize_event( FilteredEvent.state(data), self.clock.time_msec(), - config=SerializeEventConfig( + config=await self._event_serializer.create_config( event_format=EventFormat.ClientV2, requester=requester, ), @@ -925,7 +925,7 @@ class RoomMessageListRestServlet(RestServlet): ): as_client_event = False - serialize_options = SerializeEventConfig( + serialize_options = await self.event_serializer.create_config( as_client_event=as_client_event, requester=requester ) @@ -1114,7 +1114,7 @@ class RoomEventServlet(RestServlet): event, self.clock.time_msec(), bundle_aggregations=aggregations, - config=SerializeEventConfig(requester=requester), + config=await self._event_serializer.create_config(requester=requester), ) return 200, event_dict @@ -1154,7 +1154,9 @@ class RoomEventContextServlet(RestServlet): raise SynapseError(404, "Event not found.", errcode=Codes.NOT_FOUND) time_now = self.clock.time_msec() - serializer_options = SerializeEventConfig(requester=requester) + serializer_options = await self._event_serializer.create_config( + requester=requester + ) results = { "events_before": await self._event_serializer.serialize_events( event_context.events_before, diff --git a/synapse/rest/client/sync.py b/synapse/rest/client/sync.py index ec0e15c530..0488f3c8f7 100644 --- a/synapse/rest/client/sync.py +++ b/synapse/rest/client/sync.py @@ -309,12 +309,12 @@ class SyncRestServlet(RestServlet): else: raise Exception("Unknown event format %s" % (filter.event_format,)) - serialize_options = SerializeEventConfig( + serialize_options = await self._event_serializer.create_config( event_format=event_formatter, requester=requester, event_field_allowlist=filter.event_fields, ) - stripped_serialize_options = SerializeEventConfig( + stripped_serialize_options = await self._event_serializer.create_config( event_format=event_formatter, requester=requester, include_stripped_room_state=True, @@ -929,7 +929,7 @@ class SlidingSyncRestServlet(RestServlet): ) -> JsonDict: time_now = self.clock.time_msec() - serialize_options = SerializeEventConfig( + serialize_options = await self.event_serializer.create_config( event_format=EventFormat.ClientV2WithoutRoomId, requester=requester, ) diff --git a/synapse/synapse_rust/events.pyi b/synapse/synapse_rust/events.pyi index 2b87953088..fc9173acb2 100644 --- a/synapse/synapse_rust/events.pyi +++ b/synapse/synapse_rust/events.pyi @@ -364,13 +364,13 @@ class SerializeEventConfig: def __init__( self, *, - as_client_event: bool = True, - event_format: EventFormat = ..., - requester: Requester | None = None, - event_field_allowlist: list[str] | None = None, - include_stripped_room_state: bool = False, - include_admin_metadata: bool = False, - msc4354_enabled: bool = False, + as_client_event: bool, + event_format: EventFormat, + requester: Requester | None, + event_field_allowlist: list[str] | None, + include_stripped_room_state: bool, + include_admin_metadata: bool, + msc4354_enabled: bool, ) -> None: ... @property def as_client_event(self) -> bool: @@ -410,11 +410,6 @@ class SerializeEventConfig: def msc4354_enabled(self) -> bool: """Whether MSC4354 (sticky events) is enabled. When ``True``, the remaining stickiness TTL is computed and added to ``unsigned``.""" - def for_admin(self) -> SerializeEventConfig: - """Return a copy of this config with ``include_admin_metadata`` enabled.""" - - def with_msc4354(self, enabled: bool = True) -> SerializeEventConfig: - """Return a copy of this config with ``msc4354_enabled`` set as given.""" def serialize_events( events: list[tuple[Event, str | None]], diff --git a/tests/events/test_utils.py b/tests/events/test_utils.py index 669e839043..b45f7ccad0 100644 --- a/tests/events/test_utils.py +++ b/tests/events/test_utils.py @@ -26,17 +26,14 @@ from synapse.api.constants import EventContentFields from synapse.api.room_versions import RoomVersions from synapse.events import EventBase from synapse.events.utils import ( - EventFormat, FilteredEvent, PowerLevelsContent, - SerializeEventConfig, clone_event, copy_and_fixup_power_levels_contents, - make_config_for_admin, maybe_upsert_event_field, prune_event, ) -from synapse.types import JsonDict, create_requester +from synapse.types import JsonDict from synapse.util.frozenutils import freeze from tests.test_utils.event_builders import make_test_event @@ -662,9 +659,11 @@ class SerializeEventTestCase(HomeserverTestCase): self._event_serializer.serialize_event( FilteredEvent(event=ev, membership=None), 1479807801915, - config=SerializeEventConfig( - event_field_allowlist=fields, - include_admin_metadata=include_admin_metadata, + config=self.get_success( + self._event_serializer.create_config( + event_field_allowlist=fields, + include_admin_metadata=include_admin_metadata, + ) ), redaction_map=redaction_map, ) @@ -785,13 +784,19 @@ class SerializeEventTestCase(HomeserverTestCase): def test_event_fields_fail_if_fields_not_str(self) -> None: with self.assertRaises(TypeError): - SerializeEventConfig( - event_field_allowlist=["room_id", 4], # type: ignore[list-item] + self.get_success_or_raise( + self._event_serializer.create_config( + event_field_allowlist=["room_id", 4], # type: ignore[list-item] + ) ) def test_default_serialize_config_excludes_admin_metadata(self) -> None: # We just really don't want this to be set to True accidentally - self.assertFalse(SerializeEventConfig().include_admin_metadata) + self.assertFalse( + self.get_success( + self._event_serializer.create_config() + ).include_admin_metadata + ) def test_event_flagged_for_admins(self) -> None: # Default behaviour should be *not* to include it @@ -872,30 +877,6 @@ class SerializeEventTestCase(HomeserverTestCase): }, ) - def test_make_serialize_config_for_admin_retains_other_fields(self) -> None: - non_default_config = SerializeEventConfig( - include_admin_metadata=False, # should be True in a moment - as_client_event=False, # default True - event_format=EventFormat.Raw, # default EventFormat.ClientV1 - requester=create_requester("@example:example.org"), # default None - event_field_allowlist=["foo"], # default None - include_stripped_room_state=True, # default False - ) - admin_config = make_config_for_admin(non_default_config) - self.assertEqual( - admin_config.as_client_event, non_default_config.as_client_event - ) - self.assertEqual(admin_config.event_format, non_default_config.event_format) - self.assertEqual(admin_config.requester, non_default_config.requester) - self.assertEqual( - admin_config.event_field_allowlist, non_default_config.event_field_allowlist - ) - self.assertEqual( - admin_config.include_stripped_room_state, - admin_config.include_stripped_room_state, - ) - self.assertTrue(admin_config.include_admin_metadata) - def test_redacted_because_is_filtered_out(self) -> None: """If an event's unsigned dict has a `redacted_by` field, then the `redacted_because` should be filtered out if not specified in