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

Don't pull out state in compute_event_context for unconflicted state #13267

Merged
merged 8 commits into from
Jul 14, 2022
Merged
Show file tree
Hide file tree
Changes from 3 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
14 changes: 1 addition & 13 deletions synapse/state/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ def _gen_state_id() -> str:


class _StateCacheEntry:
__slots__ = ["state", "state_group", "state_id", "prev_group", "delta_ids"]
__slots__ = ["state", "state_group", "prev_group", "delta_ids"]
erikjohnston marked this conversation as resolved.
Show resolved Hide resolved

def __init__(
self,
Expand All @@ -102,18 +102,6 @@ def __init__(
self.prev_group = prev_group
self.delta_ids = frozendict(delta_ids) if delta_ids is not None else None

# The `state_id` is a unique ID we generate that can be used as ID for
# this collection of state. Usually this would be the same as the
# state group, but on worker instances we can't generate a new state
# group each time we resolve state, so we generate a separate one that
# isn't persisted and is used solely for caches.
# `state_id` is either a state_group (and so an int) or a string. This
# ensures we don't accidentally persist a state_id as a stateg_group
if state_group:
self.state_id: Union[str, int] = state_group
else:
self.state_id = _gen_state_id()

def __len__(self) -> int:
return len(self.state)

Expand Down
23 changes: 2 additions & 21 deletions synapse/storage/databases/main/roommember.py
Original file line number Diff line number Diff line change
Expand Up @@ -780,24 +780,6 @@ async def get_mutual_rooms_between_users(

return shared_room_ids or frozenset()

async def get_joined_users_from_context(
self, event: EventBase, context: EventContext
) -> Dict[str, ProfileInfo]:
state_group: Union[object, int] = context.state_group
if not state_group:
# If state_group is None it means it has yet to be assigned a
# state group, i.e. we need to make sure that calls with a state_group
# of None don't hit previous cached calls with a None state_group.
# To do this we set the state_group to a new object as object() != object()
state_group = object()

current_state_ids = await context.get_current_state_ids()
assert current_state_ids is not None
assert state_group is not None
return await self._get_joined_users_from_context(
event.room_id, state_group, current_state_ids, event=event, context=context
)

async def get_joined_users_from_state(
self, room_id: str, state_entry: "_StateCacheEntry"
) -> Dict[str, ProfileInfo]:
Expand All @@ -815,15 +797,14 @@ async def get_joined_users_from_state(
room_id, state_group, state_entry.state, context=state_entry
)

@cached(num_args=2, cache_context=True, iterable=True, max_entries=100000)
@cached(num_args=2, iterable=True, max_entries=100000)
async def _get_joined_users_from_context(
self,
room_id: str,
state_group: Union[object, int],
current_state_ids: StateMap[str],
cache_context: _CacheContext,
event: Optional[EventBase] = None,
context: Optional[Union[EventContext, "_StateCacheEntry"]] = None,
context: Optional["_StateCacheEntry"] = None,
erikjohnston marked this conversation as resolved.
Show resolved Hide resolved
) -> Dict[str, ProfileInfo]:
# We don't use `state_group`, it's there so that we can cache based
# on it. However, it's important that it's never None, since two current_states
Expand Down
54 changes: 0 additions & 54 deletions tests/storage/test_roommember.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,60 +110,6 @@ def test_count_known_servers_stat_counter_enabled(self) -> None:
# It now knows about Charlie's server.
self.assertEqual(self.store._known_servers_count, 2)

def test_get_joined_users_from_context(self) -> None:
room = self.helper.create_room_as(self.u_alice, tok=self.t_alice)
bob_event = self.get_success(
event_injection.inject_member_event(
self.hs, room, self.u_bob, Membership.JOIN
)
)

# first, create a regular event
event, context = self.get_success(
event_injection.create_event(
self.hs,
room_id=room,
sender=self.u_alice,
prev_event_ids=[bob_event.event_id],
type="m.test.1",
content={},
)
)

users = self.get_success(
self.store.get_joined_users_from_context(event, context)
)
self.assertEqual(users.keys(), {self.u_alice, self.u_bob})

# Regression test for #7376: create a state event whose key matches bob's
# user_id, but which is *not* a membership event, and persist that; then check
# that `get_joined_users_from_context` returns the correct users for the next event.
non_member_event = self.get_success(
event_injection.inject_event(
self.hs,
room_id=room,
sender=self.u_bob,
prev_event_ids=[bob_event.event_id],
type="m.test.2",
state_key=self.u_bob,
content={},
)
)
event, context = self.get_success(
event_injection.create_event(
self.hs,
room_id=room,
sender=self.u_alice,
prev_event_ids=[non_member_event.event_id],
type="m.test.3",
content={},
)
)
users = self.get_success(
self.store.get_joined_users_from_context(event, context)
)
self.assertEqual(users.keys(), {self.u_alice, self.u_bob})

def test__null_byte_in_display_name_properly_handled(self) -> None:
room = self.helper.create_room_as(self.u_alice, tok=self.t_alice)

Expand Down