From e853388b56e55291f3c07c01c66958902c2f22e7 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 17 Aug 2021 12:17:21 -0400 Subject: [PATCH 1/6] Convert GetRoomsForUserWithStreamOrdering to attrs. --- synapse/handlers/sync.py | 16 +++++++++------- synapse/storage/databases/main/roommember.py | 4 +++- synapse/storage/roommember.py | 12 +++++++++--- tests/replication/slave/storage/test_events.py | 9 ++++++--- 4 files changed, 27 insertions(+), 14 deletions(-) diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index 590642f510fe..2a7f942578b1 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -2076,21 +2076,23 @@ async def get_rooms_for_user_at( # If the membership's stream ordering is after the given stream # ordering, we need to go and work out if the user was in the room # before. - for room_id, event_pos in joined_rooms: - if not event_pos.persisted_after(room_key): - joined_room_ids.add(room_id) + for joined_room in joined_rooms: + if not joined_room.event_pos.persisted_after(room_key): + joined_room_ids.add(joined_room.room_id) continue - logger.info("User joined room after current token: %s", room_id) + logger.info("User joined room after current token: %s", joined_room.room_id) extrems = ( await self.store.get_forward_extremities_for_room_at_stream_ordering( - room_id, event_pos.stream + joined_room.room_id, joined_room.event_pos.stream ) ) - users_in_room = await self.state.get_current_users_in_room(room_id, extrems) + users_in_room = await self.state.get_current_users_in_room( + joined_room.room_id, extrems + ) if user_id in users_in_room: - joined_room_ids.add(room_id) + joined_room_ids.add(joined_room.room_id) return frozenset(joined_room_ids) diff --git a/synapse/storage/databases/main/roommember.py b/synapse/storage/databases/main/roommember.py index e8157ba3d4eb..8f5409957789 100644 --- a/synapse/storage/databases/main/roommember.py +++ b/synapse/storage/databases/main/roommember.py @@ -522,7 +522,9 @@ def _get_users_server_still_shares_room_with_txn(txn): _get_users_server_still_shares_room_with_txn, ) - async def get_rooms_for_user(self, user_id: str, on_invalidate=None): + async def get_rooms_for_user( + self, user_id: str, on_invalidate=None + ) -> FrozenSet[str]: """Returns a set of room_ids the user is currently joined to. If a remote user only returns rooms this server is currently diff --git a/synapse/storage/roommember.py b/synapse/storage/roommember.py index c34fbf21bc42..9d1b2999d103 100644 --- a/synapse/storage/roommember.py +++ b/synapse/storage/roommember.py @@ -16,6 +16,10 @@ import logging from collections import namedtuple +import attr + +from synapse.types import PersistedEventPosition + logger = logging.getLogger(__name__) @@ -23,9 +27,11 @@ "RoomsForUser", ("room_id", "sender", "membership", "event_id", "stream_ordering") ) -GetRoomsForUserWithStreamOrdering = namedtuple( - "GetRoomsForUserWithStreamOrdering", ("room_id", "event_pos") -) + +@attr.s(slots=True, frozen=True, auto_attribs=True) +class GetRoomsForUserWithStreamOrdering: + room_id: str + event_pos: PersistedEventPosition # We store this using a namedtuple so that we save about 3x space over using a diff --git a/tests/replication/slave/storage/test_events.py b/tests/replication/slave/storage/test_events.py index db80a0bdbdaf..8d1b0606c469 100644 --- a/tests/replication/slave/storage/test_events.py +++ b/tests/replication/slave/storage/test_events.py @@ -20,7 +20,7 @@ from synapse.events import FrozenEvent, _EventInternalMetadata, make_event_from_dict from synapse.handlers.room import RoomEventSource from synapse.replication.slave.storage.events import SlavedEventStore -from synapse.storage.roommember import RoomsForUser +from synapse.storage.roommember import GetRoomsForUserWithStreamOrdering, RoomsForUser from synapse.types import PersistedEventPosition from tests.server import FakeTransport @@ -216,7 +216,7 @@ def test_get_rooms_for_user_with_stream_ordering(self): self.check( "get_rooms_for_user_with_stream_ordering", (USER_ID_2,), - {(ROOM_ID, expected_pos)}, + {GetRoomsForUserWithStreamOrdering(ROOM_ID, expected_pos)}, ) def test_get_rooms_for_user_with_stream_ordering_with_multi_event_persist(self): @@ -305,7 +305,10 @@ def test_get_rooms_for_user_with_stream_ordering_with_multi_event_persist(self): expected_pos = PersistedEventPosition( "master", j2.internal_metadata.stream_ordering ) - self.assertEqual(joined_rooms, {(ROOM_ID, expected_pos)}) + self.assertEqual( + joined_rooms, + {GetRoomsForUserWithStreamOrdering(ROOM_ID, expected_pos)}, + ) event_id = 0 From efa615b3016b6bff7072aec331c9bf7bafc1422b Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 17 Aug 2021 12:23:19 -0400 Subject: [PATCH 2/6] Convert RoomsForUser to attrs. --- synapse/handlers/initial_sync.py | 2 +- synapse/storage/databases/main/roommember.py | 4 +++- synapse/storage/roommember.py | 10 +++++++--- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/synapse/handlers/initial_sync.py b/synapse/handlers/initial_sync.py index e1c544a3c981..4e8f7f1d8553 100644 --- a/synapse/handlers/initial_sync.py +++ b/synapse/handlers/initial_sync.py @@ -151,7 +151,7 @@ async def _snapshot_all_rooms( limit = 10 async def handle_room(event: RoomsForUser): - d = { + d: JsonDict = { "room_id": event.room_id, "membership": event.membership, "visibility": ( diff --git a/synapse/storage/databases/main/roommember.py b/synapse/storage/databases/main/roommember.py index 8f5409957789..c2f6b9d63d70 100644 --- a/synapse/storage/databases/main/roommember.py +++ b/synapse/storage/databases/main/roommember.py @@ -307,7 +307,9 @@ def _get_room_summary_txn(txn): ) @cached() - async def get_invited_rooms_for_local_user(self, user_id: str) -> RoomsForUser: + async def get_invited_rooms_for_local_user( + self, user_id: str + ) -> List[RoomsForUser]: """Get all the rooms the *local* user is invited to. Args: diff --git a/synapse/storage/roommember.py b/synapse/storage/roommember.py index 9d1b2999d103..f65f01e52f10 100644 --- a/synapse/storage/roommember.py +++ b/synapse/storage/roommember.py @@ -23,9 +23,13 @@ logger = logging.getLogger(__name__) -RoomsForUser = namedtuple( - "RoomsForUser", ("room_id", "sender", "membership", "event_id", "stream_ordering") -) +@attr.s(slots=True, frozen=True, auto_attribs=True) +class RoomsForUser: + room_id: str + sender: str + membership: str + event_id: str + stream_ordering: int @attr.s(slots=True, frozen=True, auto_attribs=True) From 9d4d3a1715dc3d71e54ce5a15cf66647123943a1 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 17 Aug 2021 12:30:52 -0400 Subject: [PATCH 3/6] Convert ProfileInfo to attrs. --- synapse/storage/databases/main/user_directory.py | 2 +- synapse/storage/roommember.py | 9 ++++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/synapse/storage/databases/main/user_directory.py b/synapse/storage/databases/main/user_directory.py index 9d28d69ac7d1..65dde67ae98d 100644 --- a/synapse/storage/databases/main/user_directory.py +++ b/synapse/storage/databases/main/user_directory.py @@ -365,7 +365,7 @@ async def is_room_world_readable_or_publicly_joinable(self, room_id): return False async def update_profile_in_user_dir( - self, user_id: str, display_name: str, avatar_url: str + self, user_id: str, display_name: Optional[str], avatar_url: Optional[str] ) -> None: """ Update or add a user's profile in the user directory. diff --git a/synapse/storage/roommember.py b/synapse/storage/roommember.py index f65f01e52f10..406cf4a4742d 100644 --- a/synapse/storage/roommember.py +++ b/synapse/storage/roommember.py @@ -15,6 +15,7 @@ import logging from collections import namedtuple +from typing import Optional import attr @@ -38,9 +39,11 @@ class GetRoomsForUserWithStreamOrdering: event_pos: PersistedEventPosition -# We store this using a namedtuple so that we save about 3x space over using a -# dict. -ProfileInfo = namedtuple("ProfileInfo", ("avatar_url", "display_name")) +@attr.s(slots=True, frozen=True, auto_attribs=True) +class ProfileInfo: + avatar_url: Optional[str] + display_name: Optional[str] + # "members" points to a truncated list of (user_id, event_id) tuples for users of # a given membership type, suitable for use in calculating heroes for a room. From 4b1002cb9beaebdc1367921098c6f6381f58ad71 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 17 Aug 2021 13:03:53 -0400 Subject: [PATCH 4/6] Convert MemberSummary to attrs. --- synapse/handlers/sync.py | 2 +- synapse/storage/roommember.py | 14 ++++++++------ 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index 2a7f942578b1..d08039ddb033 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -701,7 +701,7 @@ async def compute_summary( name_id = state_ids.get((EventTypes.Name, "")) canonical_alias_id = state_ids.get((EventTypes.CanonicalAlias, "")) - summary = {} + summary: JsonDict = {} empty_ms = MemberSummary([], 0) # TODO: only send these when they change. diff --git a/synapse/storage/roommember.py b/synapse/storage/roommember.py index 406cf4a4742d..dd2d1df784f0 100644 --- a/synapse/storage/roommember.py +++ b/synapse/storage/roommember.py @@ -14,8 +14,7 @@ # limitations under the License. import logging -from collections import namedtuple -from typing import Optional +from typing import List, Optional, Tuple import attr @@ -45,7 +44,10 @@ class ProfileInfo: display_name: Optional[str] -# "members" points to a truncated list of (user_id, event_id) tuples for users of -# a given membership type, suitable for use in calculating heroes for a room. -# "count" points to the total numberr of users of a given membership type. -MemberSummary = namedtuple("MemberSummary", ("members", "count")) +@attr.s(slots=True, frozen=True, auto_attribs=True) +class MemberSummary: + # A truncated list of (user_id, event_id) tuples for users of a given + # membership type, suitable for use in calculating heroes for a room. + members: List[Tuple[str, str]] + # The total number of users of a given membership type. + count: int From 3f8b313c02f6a33310f91ee4897bf21083ecf0e4 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 17 Aug 2021 13:46:14 -0400 Subject: [PATCH 5/6] Newsfragment --- changelog.d/10629.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/10629.misc diff --git a/changelog.d/10629.misc b/changelog.d/10629.misc new file mode 100644 index 000000000000..cca1eb6c5711 --- /dev/null +++ b/changelog.d/10629.misc @@ -0,0 +1 @@ +Convert room member storage tuples to `attrs` classes. From e97ac25282d763c438221d462ece249f0e21a757 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 18 Aug 2021 07:49:20 -0400 Subject: [PATCH 6/6] Use weakref_slot. --- synapse/storage/roommember.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/synapse/storage/roommember.py b/synapse/storage/roommember.py index dd2d1df784f0..0ff66debdfcc 100644 --- a/synapse/storage/roommember.py +++ b/synapse/storage/roommember.py @@ -23,7 +23,7 @@ logger = logging.getLogger(__name__) -@attr.s(slots=True, frozen=True, auto_attribs=True) +@attr.s(slots=True, frozen=True, weakref_slot=True, auto_attribs=True) class RoomsForUser: room_id: str sender: str @@ -32,19 +32,19 @@ class RoomsForUser: stream_ordering: int -@attr.s(slots=True, frozen=True, auto_attribs=True) +@attr.s(slots=True, frozen=True, weakref_slot=True, auto_attribs=True) class GetRoomsForUserWithStreamOrdering: room_id: str event_pos: PersistedEventPosition -@attr.s(slots=True, frozen=True, auto_attribs=True) +@attr.s(slots=True, frozen=True, weakref_slot=True, auto_attribs=True) class ProfileInfo: avatar_url: Optional[str] display_name: Optional[str] -@attr.s(slots=True, frozen=True, auto_attribs=True) +@attr.s(slots=True, frozen=True, weakref_slot=True, auto_attribs=True) class MemberSummary: # A truncated list of (user_id, event_id) tuples for users of a given # membership type, suitable for use in calculating heroes for a room.