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

Refactor EventContext #12689

Merged
merged 9 commits into from
May 10, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
56 changes: 18 additions & 38 deletions synapse/events/snapshot.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ class EventContext:
If ``state_group`` is None (ie, the event is an outlier),
``state_group_before_event`` will always also be ``None``.

delta_before_after: If `state_group` and `state_group_before_event` are not None
then this is the delta of the state between the two groups.
Copy link
Member

Choose a reason for hiding this comment

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

Is there a better name we can use? Maybe state_delta_at_event or something?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's hard because I want to make sure people don't think that its the state delta since the prev_group

Copy link
Member

Choose a reason for hiding this comment

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

Right! Just the before_after I find confusing. Before/after what?!

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh i see. Yes. Ermh, delta_before_and_after_event?


prev_group: If it is known, ``state_group``'s prev_group. Note that this being
None does not necessarily mean that ``state_group`` does not have
a prev_group!
Expand All @@ -75,30 +78,6 @@ class EventContext:
app_service: If this event is being sent by a (local) application service, that
app service.

_current_state_ids: The room state map, including this event - ie, the state
in ``state_group``.

(type, state_key) -> event_id

For an outlier, this is {}

Note that this is a private attribute: it should be accessed via
``get_current_state_ids``. _AsyncEventContext impl calculates this
on-demand: it will be None until that happens.

_prev_state_ids: The room state map, excluding this event - ie, the state
in ``state_group_before_event``. For a non-state
event, this will be the same as _current_state_events.

Note that it is a completely different thing to prev_group!

(type, state_key) -> event_id

For an outlier, this is {}

As with _current_state_ids, this is a private attribute. It should be
accessed via get_prev_state_ids.

partial_state: if True, we may be storing this event with a temporary,
incomplete state.
"""
Expand All @@ -107,22 +86,19 @@ class EventContext:
rejected: Union[bool, str] = False
_state_group: Optional[int] = None
state_group_before_event: Optional[int] = None
_delta_before_after: Optional[StateMap[str]] = None
prev_group: Optional[int] = None
delta_ids: Optional[StateMap[str]] = None
app_service: Optional[ApplicationService] = None

_current_state_ids: Optional[StateMap[str]] = None
_prev_state_ids: Optional[StateMap[str]] = None

partial_state: bool = False

@staticmethod
def with_state(
storage: "Storage",
state_group: Optional[int],
state_group_before_event: Optional[int],
current_state_ids: Optional[StateMap[str]],
prev_state_ids: Optional[StateMap[str]],
delta_before_after: Optional[StateMap[str]],
partial_state: bool,
prev_group: Optional[int] = None,
delta_ids: Optional[StateMap[str]] = None,
Expand All @@ -131,6 +107,7 @@ def with_state(
storage=storage,
state_group=state_group,
state_group_before_event=state_group_before_event,
delta_before_after=delta_before_after,
prev_group=prev_group,
delta_ids=delta_ids,
partial_state=partial_state,
Expand All @@ -141,11 +118,7 @@ def for_outlier(
storage: "Storage",
) -> "EventContext":
"""Return an EventContext instance suitable for persisting an outlier event"""
return EventContext(
storage=storage,
current_state_ids={},
prev_state_ids={},
)
return EventContext(storage=storage)

async def serialize(self, event: EventBase, store: "DataStore") -> JsonDict:
"""Converts self to a type that can be serialized as JSON, and then
Expand All @@ -163,6 +136,7 @@ async def serialize(self, event: EventBase, store: "DataStore") -> JsonDict:
"state_group_before_event": self.state_group_before_event,
"rejected": self.rejected,
"prev_group": self.prev_group,
"delta_before_after": _encode_state_dict(self._delta_before_after),
"delta_ids": _encode_state_dict(self.delta_ids),
"app_service_id": self.app_service.id if self.app_service else None,
"partial_state": self.partial_state,
Expand All @@ -187,6 +161,7 @@ def deserialize(storage: "Storage", input: JsonDict) -> "EventContext":
state_group=input["state_group"],
state_group_before_event=input["state_group_before_event"],
prev_group=input["prev_group"],
delta_before_after=_decode_state_dict(input["delta_before_after"]),
delta_ids=_decode_state_dict(input["delta_ids"]),
rejected=input["rejected"],
partial_state=input.get("partial_state", False),
Expand Down Expand Up @@ -234,10 +209,15 @@ async def get_current_state_ids(self) -> Optional[StateMap[str]]:
if self.rejected:
raise RuntimeError("Attempt to access state_ids of rejected event")

if self._state_group is None:
return None
assert self._delta_before_after is not None

prev_state_ids = await self.get_prev_state_ids()

if self._delta_before_after:
prev_state_ids = dict(prev_state_ids)
prev_state_ids.update(self._delta_before_after)

return await self._storage.state.get_state_ids_for_group(self._state_group)
return prev_state_ids

async def get_prev_state_ids(self) -> StateMap[str]:
"""
Expand All @@ -252,7 +232,7 @@ async def get_prev_state_ids(self) -> StateMap[str]:
Maps a (type, state_key) to the event ID of the state event matching
this tuple.
"""
assert self.state_group_before_event
assert self.state_group_before_event is not None
return await self._storage.state.get_state_ids_for_group(
self.state_group_before_event
)
Expand Down
3 changes: 1 addition & 2 deletions synapse/handlers/federation_event.py
Original file line number Diff line number Diff line change
Expand Up @@ -1877,8 +1877,7 @@ async def _update_context_for_auth_events(
storage=self._storage,
state_group=state_group,
state_group_before_event=context.state_group_before_event,
current_state_ids=current_state_ids,
prev_state_ids=prev_state_ids,
delta_before_after=state_updates,
prev_group=prev_group,
delta_ids=state_updates,
partial_state=context.partial_state,
Expand Down
4 changes: 4 additions & 0 deletions synapse/handlers/message.py
Original file line number Diff line number Diff line change
Expand Up @@ -757,6 +757,10 @@ async def deduplicate_state_event(
The previous version of the event is returned, if it is found in the
event context. Otherwise, None is returned.
"""
if event.internal_metadata.is_outlier():
# This can happen due to out of band memberships
return None

prev_state_ids = await context.get_prev_state_ids()
prev_event_id = prev_state_ids.get((event.type, event.state_key))
if not prev_event_id:
Expand Down
4 changes: 4 additions & 0 deletions synapse/push/action_generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,5 +40,9 @@ def __init__(self, hs: "HomeServer"):
async def handle_push_actions_for_event(
self, event: EventBase, context: EventContext
) -> None:
if event.internal_metadata.is_outlier():
# This can happen due to out of band memberships
return

with Measure(self.clock, "action_for_event_by_user"):
await self.bulk_evaluator.action_for_event_by_user(event, context)
6 changes: 2 additions & 4 deletions synapse/state/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -365,8 +365,7 @@ async def compute_event_context(
storage=self._storage,
state_group_before_event=state_group_before_event,
state_group=state_group_before_event,
current_state_ids=state_ids_before_event,
prev_state_ids=state_ids_before_event,
delta_before_after={},
prev_group=state_group_before_event_prev_group,
delta_ids=deltas_to_state_group_before_event,
partial_state=partial_state,
Expand Down Expand Up @@ -398,8 +397,7 @@ async def compute_event_context(
storage=self._storage,
state_group=state_group_after_event,
state_group_before_event=state_group_before_event,
current_state_ids=state_ids_after_event,
prev_state_ids=state_ids_before_event,
delta_before_after=delta_ids,
prev_group=state_group_before_event,
delta_ids=delta_ids,
partial_state=partial_state,
Expand Down