From afeed5bec2668df88663685a6be5fc1aaca3395c Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Tue, 7 Jan 2025 11:53:38 -0800 Subject: [PATCH 01/24] add column `participant` and background update to populate it --- synapse/storage/databases/main/roommember.py | 75 +++++++++++++++++++ synapse/storage/schema/__init__.py | 5 +- ...umn_participant_room_memberships_table.sql | 20 +++++ 3 files changed, 99 insertions(+), 1 deletion(-) create mode 100644 synapse/storage/schema/main/delta/89/01_add_column_participant_room_memberships_table.sql diff --git a/synapse/storage/databases/main/roommember.py b/synapse/storage/databases/main/roommember.py index 50ed6a28bf0..fa596762d35 100644 --- a/synapse/storage/databases/main/roommember.py +++ b/synapse/storage/databases/main/roommember.py @@ -1636,6 +1636,81 @@ def __init__( columns=["user_id", "room_id"], ) + self.db_pool.updates.register_background_update_handler( + "populate_participant_bg_update", self._populate_participant + ) + + async def _populate_participant(self, progress: JsonDict, batch_size: int) -> int: + """ + Background update to populate column `participant` on `room_memberships` table + one room at a time + """ + last_room_id = progress.get("last_room_id", "") + + def _get_current_room_txn( + txn: LoggingTransaction, last_room_id: str + ) -> Optional[str]: + sql = """ + SELECT room_id from room_memberships WHERE room_id > ? + ORDER BY room_id + LIMIT 1; + """ + txn.execute(sql, (last_room_id,)) + res = txn.fetchone() + if res: + room_id = res[0] + return room_id + else: + return None + + def _background_populate_participant_per_room_txn( + txn: LoggingTransaction, current_room_id: str + ) -> None: + sql = """ + SELECT DISTINCT rm.user_id + FROM room_memberships AS rm + INNER JOIN events AS e USING(room_id) + WHERE room_id = ? + AND rm.membership = 'join' + AND e.type = 'm.room.message' + AND rm.user_id = e.sender; + """ + + txn.execute(sql, (current_room_id,)) + participants = txn.fetchall() + + for participant in participants: + self.db_pool.simple_update_txn( + txn, + table="room_memberships", + keyvalues={"user_id": participant}, + updatevalues={"participant": True}, + ) + + current_room_id = await self.db_pool.runInteraction( + "_get_current_room_txn", _get_current_room_txn, last_room_id + ) + if not current_room_id: + await self.db_pool.updates._end_background_update( + "populate_participant_bg_update" + ) + return 1 + + await self.db_pool.runInteraction( + "_background_populate_participant_per_room_txn", + _background_populate_participant_per_room_txn, + current_room_id, + ) + + progress["last_room_id"] = current_room_id + await self.db_pool.runInteraction( + "populate_participant_bg_update", + self.db_pool.updates._background_update_progress_txn, + "populate_participant_bg_update", + progress, + ) + return 1 + async def _background_add_membership_profile( self, progress: JsonDict, batch_size: int ) -> int: diff --git a/synapse/storage/schema/__init__.py b/synapse/storage/schema/__init__.py index 934e1cccedb..67a5c18e90c 100644 --- a/synapse/storage/schema/__init__.py +++ b/synapse/storage/schema/__init__.py @@ -19,7 +19,7 @@ # # -SCHEMA_VERSION = 88 # remember to update the list below when updating +SCHEMA_VERSION = 89 # remember to update the list below when updating """Represents the expectations made by the codebase about the database schema This should be incremented whenever the codebase changes its requirements on the @@ -155,6 +155,9 @@ be posted in response to a resettable timeout or an on-demand action. - Add background update to fix data integrity issue in the `sliding_sync_membership_snapshots` -> `forgotten` column + +Changes in SCHEMA_VERSION = 89 + - Add a column `participant` to `room_memberships` table """ diff --git a/synapse/storage/schema/main/delta/89/01_add_column_participant_room_memberships_table.sql b/synapse/storage/schema/main/delta/89/01_add_column_participant_room_memberships_table.sql new file mode 100644 index 00000000000..3d8ee437137 --- /dev/null +++ b/synapse/storage/schema/main/delta/89/01_add_column_participant_room_memberships_table.sql @@ -0,0 +1,20 @@ +-- +-- This file is licensed under the Affero General Public License (AGPL) version 3. +-- +-- Copyright (C) 2025 New Vector, Ltd +-- +-- This program is free software: you can redistribute it and/or modify +-- it under the terms of the GNU Affero General Public License as +-- published by the Free Software Foundation, either version 3 of the +-- License, or (at your option) any later version. +-- +-- See the GNU Affero General Public License for more details: +-- . + +-- Add a column `participant` to `room_memberships` table to track whether a room member has sent +-- a `m.room.message` event into a room they are a member of +ALTER TABLE room_memberships ADD COLUMN participant BOOLEAN DEFAULT FALSE; + +-- Add a background update to populate `participant` column +INSERT INTO background_updates (ordering, update_name, progress_json) VALUES + (8901, 'populate_participant_bg_update', '{}'); \ No newline at end of file From 754c27b0d42d9a9226904fd1af5f364e110e1fd8 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Tue, 7 Jan 2025 11:54:12 -0800 Subject: [PATCH 02/24] set participant column when sending messages --- synapse/handlers/message.py | 3 ++ synapse/storage/databases/main/roommember.py | 29 ++++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index df3010ecf68..2ea2f1a3bd3 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -1440,6 +1440,9 @@ async def handle_new_client_event( ) return prev_event + if event.type == "m.room.message": + await self.store.set_room_participation(event.room_id, event.user_id) + if event.internal_metadata.is_out_of_band_membership(): # the only sort of out-of-band-membership events we expect to see here are # invite rejections and rescinded knocks that we have generated ourselves. diff --git a/synapse/storage/databases/main/roommember.py b/synapse/storage/databases/main/roommember.py index fa596762d35..b80ddcd3baa 100644 --- a/synapse/storage/databases/main/roommember.py +++ b/synapse/storage/databases/main/roommember.py @@ -1606,6 +1606,35 @@ def _get_rooms_for_user_by_join_date_txn( from_ts, ) + async def set_room_participation(self, room_id: str, user_id: str) -> None: + """ + Record the provided user as participating in the given room + + Args: + room_id: ID of the room to set the participant in + user_id: the user ID of the user + """ + await self.db_pool.simple_update( + "room_memberships", + {"user_id": user_id, "room_id": room_id}, + {"participant": True}, + "update_room_participation", + ) + + async def get_room_participation(self, room_id: str, user_id: str) -> bool: + """ + Check whether a user is listed as a participant in a room + + Args: + room_id: ID of the room to check in + user_id: user ID of the user + """ + return await self.db_pool.simple_select_one_onecol( + "room_memberships", + {"user_id": user_id, "room_id": room_id}, + "participant", + ) + class RoomMemberBackgroundUpdateStore(SQLBaseStore): def __init__( From 830e5e6a1e9a7bfde56d398cc0f889996b06ffa7 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Tue, 7 Jan 2025 11:54:17 -0800 Subject: [PATCH 03/24] test --- tests/rest/client/test_rooms.py | 47 +++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py index 4cf1a3dc519..7c634e9fbc0 100644 --- a/tests/rest/client/test_rooms.py +++ b/tests/rest/client/test_rooms.py @@ -4070,3 +4070,50 @@ def test_suspended_user_cannot_redact_messages_other_than_their_own(self) -> Non shorthand=False, ) self.assertEqual(channel.code, 200) + + +class RoomParticipantTestCase(unittest.HomeserverTestCase): + servlets = [ + login.register_servlets, + room.register_servlets, + profile.register_servlets, + admin.register_servlets, + ] + + def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: + self.user1 = self.register_user("thomas", "hackme") + self.tok1 = self.login("thomas", "hackme") + + self.user2 = self.register_user("teresa", "hackme") + self.tok2 = self.login("teresa", "hackme") + + self.room1 = self.helper.create_room_as(room_creator=self.user1, tok=self.tok1) + self.store = hs.get_datastores().main + + def test_sending_message_records_participation(self) -> None: + """ + Test that sending an m.room.message event into a room causes the user to + be marked as a participant in that room + """ + self.helper.join(self.room1, self.user2, tok=self.tok2) + + # user has not sent any messages, so should not be a participant + participant = self.get_success( + self.store.get_room_participation(self.room1, self.user2) + ) + self.assertFalse(participant) + + # sending a message should now mark user as participant + self.helper.send_event( + self.room1, + "m.room.message", + content={ + "msgtype": "m.text", + "body": "I am engaging in this room", + }, + tok=self.tok2, + ) + participant = self.get_success( + self.store.get_room_participation(self.room1, self.user2) + ) + self.assertTrue(participant) From bc05f58bbe5ff3c27165fe130b2a45cf071118e2 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Tue, 7 Jan 2025 12:07:37 -0800 Subject: [PATCH 04/24] newsfragment --- changelog.d/18068.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/18068.misc diff --git a/changelog.d/18068.misc b/changelog.d/18068.misc new file mode 100644 index 00000000000..af6f78f5492 --- /dev/null +++ b/changelog.d/18068.misc @@ -0,0 +1 @@ +Add a column `participant` to `room_memberships` table. \ No newline at end of file From e76f94e96cb795781ca94f852d6ceebe017ed241 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Tue, 7 Jan 2025 12:24:31 -0800 Subject: [PATCH 05/24] add `participant` to list of boolean columns in port_db script --- synapse/_scripts/synapse_port_db.py | 1 + 1 file changed, 1 insertion(+) diff --git a/synapse/_scripts/synapse_port_db.py b/synapse/_scripts/synapse_port_db.py index d8f6f8ebdc3..1144ee470de 100755 --- a/synapse/_scripts/synapse_port_db.py +++ b/synapse/_scripts/synapse_port_db.py @@ -128,6 +128,7 @@ "pushers": ["enabled"], "redactions": ["have_censored"], "remote_media_cache": ["authenticated"], + "room_memberships": ["participant"], "room_stats_state": ["is_federatable"], "rooms": ["is_public", "has_auth_chain_index"], "sliding_sync_joined_rooms": ["is_encrypted"], From 62e6a04c146db0a8c92aecdff585fba0b9058a00 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Tue, 7 Jan 2025 13:07:45 -0800 Subject: [PATCH 06/24] use digit for boolean --- synapse/storage/databases/main/roommember.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/storage/databases/main/roommember.py b/synapse/storage/databases/main/roommember.py index b80ddcd3baa..3fde7d23205 100644 --- a/synapse/storage/databases/main/roommember.py +++ b/synapse/storage/databases/main/roommember.py @@ -1617,7 +1617,7 @@ async def set_room_participation(self, room_id: str, user_id: str) -> None: await self.db_pool.simple_update( "room_memberships", {"user_id": user_id, "room_id": room_id}, - {"participant": True}, + {"participant": 1}, "update_room_participation", ) @@ -1713,7 +1713,7 @@ def _background_populate_participant_per_room_txn( txn, table="room_memberships", keyvalues={"user_id": participant}, - updatevalues={"participant": True}, + updatevalues={"participant": 1}, ) current_room_id = await self.db_pool.runInteraction( From 93e7a0cbf02dc012227e89bd8c95b9fff3534926 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Tue, 7 Jan 2025 13:52:43 -0800 Subject: [PATCH 07/24] fix actual issue --- synapse/storage/databases/main/roommember.py | 23 +++++++++++--------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/synapse/storage/databases/main/roommember.py b/synapse/storage/databases/main/roommember.py index 3fde7d23205..6b8073c1804 100644 --- a/synapse/storage/databases/main/roommember.py +++ b/synapse/storage/databases/main/roommember.py @@ -1617,7 +1617,7 @@ async def set_room_participation(self, room_id: str, user_id: str) -> None: await self.db_pool.simple_update( "room_memberships", {"user_id": user_id, "room_id": room_id}, - {"participant": 1}, + {"participant": True}, "update_room_participation", ) @@ -1706,15 +1706,18 @@ def _background_populate_participant_per_room_txn( """ txn.execute(sql, (current_room_id,)) - participants = txn.fetchall() - - for participant in participants: - self.db_pool.simple_update_txn( - txn, - table="room_memberships", - keyvalues={"user_id": participant}, - updatevalues={"participant": 1}, - ) + res = txn.fetchall() + + if res: + participants = [user[0] for user in res] + + for participant in participants: + self.db_pool.simple_update_txn( + txn, + table="room_memberships", + keyvalues={"user_id": participant}, + updatevalues={"participant": True}, + ) current_room_id = await self.db_pool.runInteraction( "_get_current_room_txn", _get_current_room_txn, last_room_id From 115ff0061b9872a1814b91544f4b0662db69f4c9 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Sun, 12 Jan 2025 20:01:17 -0800 Subject: [PATCH 08/24] use current_state_events + capture encrypted messages --- synapse/handlers/message.py | 2 +- synapse/storage/databases/main/roommember.py | 12 +++---- tests/rest/client/test_rooms.py | 34 ++++++++++++++++++++ 3 files changed, 41 insertions(+), 7 deletions(-) diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index 2ea2f1a3bd3..0a1ecdf5616 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -1440,7 +1440,7 @@ async def handle_new_client_event( ) return prev_event - if event.type == "m.room.message": + if event.type == "m.room.message" or event.type == "m.room.encrypted": await self.store.set_room_participation(event.room_id, event.user_id) if event.internal_metadata.is_out_of_band_membership(): diff --git a/synapse/storage/databases/main/roommember.py b/synapse/storage/databases/main/roommember.py index 6b8073c1804..42d9ad2d5db 100644 --- a/synapse/storage/databases/main/roommember.py +++ b/synapse/storage/databases/main/roommember.py @@ -1696,13 +1696,14 @@ def _background_populate_participant_per_room_txn( txn: LoggingTransaction, current_room_id: str ) -> None: sql = """ - SELECT DISTINCT rm.user_id - FROM room_memberships AS rm + SELECT DISTINCT c.state_key + FROM current_state_events AS c INNER JOIN events AS e USING(room_id) WHERE room_id = ? - AND rm.membership = 'join' + AND c.membership = 'join' AND e.type = 'm.room.message' - AND rm.user_id = e.sender; + OR e.type = 'm.room.encrypted' + AND c.state_key = e.sender; """ txn.execute(sql, (current_room_id,)) @@ -1710,12 +1711,11 @@ def _background_populate_participant_per_room_txn( if res: participants = [user[0] for user in res] - for participant in participants: self.db_pool.simple_update_txn( txn, table="room_memberships", - keyvalues={"user_id": participant}, + keyvalues={"user_id": participant, "room_id": current_room_id}, updatevalues={"participant": True}, ) diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py index 7c634e9fbc0..ba0ff4d0451 100644 --- a/tests/rest/client/test_rooms.py +++ b/tests/rest/client/test_rooms.py @@ -4088,6 +4088,9 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: self.tok2 = self.login("teresa", "hackme") self.room1 = self.helper.create_room_as(room_creator=self.user1, tok=self.tok1) + self.room2 = self.helper.create_room_as( + self.user1, is_public=False, tok=self.tok1 + ) self.store = hs.get_datastores().main def test_sending_message_records_participation(self) -> None: @@ -4117,3 +4120,34 @@ def test_sending_message_records_participation(self) -> None: self.store.get_room_participation(self.room1, self.user2) ) self.assertTrue(participant) + + def test_sending_encrypted_event_records_participation(self) -> None: + """ + Test that sending an m.room.encrypted event into a room causes the user to + be marked as a participant in that room + """ + self.helper.join(self.room1, self.user2, tok=self.tok2) + + # user has not sent any messages, so should not be a participant + participant = self.get_success( + self.store.get_room_participation(self.room1, self.user2) + ) + self.assertFalse(participant) + + # sending an encrypted event should now mark user as participant + self.helper.send_event( + self.room1, + "m.room.encrypted", + content={ + "algorithm": "m.megolm.v1.aes-sha2", + "ciphertext": "AwgAEnACgAkLmt6qF84IK++J7UDH2Za1YVchHyprqTqsg...", + "device_id": "RJYKSTBOIE", + "sender_key": "IlRMeOPX2e0MurIyfWEucYBRVOEEUMrOHqn/8mLqMjA", + "session_id": "X3lUlvLELLYxeTx4yOVu6UDpasGEVO0Jbu+QFnm0cKQ", + }, + tok=self.tok2, + ) + participant = self.get_success( + self.store.get_room_participation(self.room1, self.user2) + ) + self.assertTrue(participant) From b0f804e5800c7174fe2c27117bf9b7a141be3fbc Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Wed, 29 Jan 2025 16:46:03 -0800 Subject: [PATCH 09/24] requested changes --- synapse/storage/databases/main/roommember.py | 136 ++++++++++-------- ...umn_participant_room_memberships_table.sql | 2 +- 2 files changed, 81 insertions(+), 57 deletions(-) diff --git a/synapse/storage/databases/main/roommember.py b/synapse/storage/databases/main/roommember.py index 42d9ad2d5db..7ab879fc134 100644 --- a/synapse/storage/databases/main/roommember.py +++ b/synapse/storage/databases/main/roommember.py @@ -1606,22 +1606,33 @@ def _get_rooms_for_user_by_join_date_txn( from_ts, ) - async def set_room_participation(self, room_id: str, user_id: str) -> None: + async def set_room_participation(self, user_id: str, room_id: str) -> None: """ Record the provided user as participating in the given room Args: - room_id: ID of the room to set the participant in user_id: the user ID of the user + room_id: ID of the room to set the participant in """ - await self.db_pool.simple_update( - "room_memberships", - {"user_id": user_id, "room_id": room_id}, - {"participant": True}, - "update_room_participation", + + def _set_room_participation_txn( + txn: LoggingTransaction, user_id: str, room_id: str + ) -> None: + sql = """ + UPDATE room_memberships + SET participant = True + WHERE user_id = ? + AND room_id = ? + ORDER BY event_stream_ordering DESC + LIMIT 1 + """ + txn.execute(sql, (user_id, room_id)) + + await self.db_pool.runInteraction( + "_set_room_participation_txn", _set_room_participation_txn, user_id, room_id ) - async def get_room_participation(self, room_id: str, user_id: str) -> bool: + async def get_room_participation(self, user_id: str, room_id: str) -> bool: """ Check whether a user is listed as a participant in a room @@ -1629,10 +1640,26 @@ async def get_room_participation(self, room_id: str, user_id: str) -> bool: room_id: ID of the room to check in user_id: user ID of the user """ - return await self.db_pool.simple_select_one_onecol( - "room_memberships", - {"user_id": user_id, "room_id": room_id}, - "participant", + + def _get_room_participation_txn( + txn: LoggingTransaction, user_id: str, room_id: str + ) -> bool: + sql = """ + SELECT participant + FROM room_memberships + WHERE user_id = ? + AND room_id = ? + ORDER BY event_stream_ordering DESC + LIMIT 1 + """ + txn.execute(sql, (user_id, room_id)) + res = txn.fetchone() + if res: + return res[0] + return False + + return await self.db_pool.runInteraction( + "_get_room_participation_txn", _get_room_participation_txn, user_id, room_id ) @@ -1672,76 +1699,73 @@ def __init__( async def _populate_participant(self, progress: JsonDict, batch_size: int) -> int: """ Background update to populate column `participant` on `room_memberships` table - one room at a time + + A 'participant' is someone who is currently joined to a room and has sent at least + one `m.room.message` or `m.room.encrypted` event. """ - last_room_id = progress.get("last_room_id", "") + stream_token = progress.get("last_stream_token", None) - def _get_current_room_txn( - txn: LoggingTransaction, last_room_id: str - ) -> Optional[str]: + def _get_max_stream_token_txn(txn: LoggingTransaction) -> Optional[str]: sql = """ - SELECT room_id from room_memberships WHERE room_id > ? - ORDER BY room_id + SELECT event_stream_ordering from room_memberships + ORDER BY event_stream_ordering DESC LIMIT 1; """ - txn.execute(sql, (last_room_id,)) + txn.execute(sql) res = txn.fetchone() - if res: - room_id = res[0] - return room_id - else: - return None + assert res is not None + return res[0] - def _background_populate_participant_per_room_txn( - txn: LoggingTransaction, current_room_id: str + def _background_populate_participant_txn( + txn: LoggingTransaction, stream_token: str ) -> None: sql = """ - SELECT DISTINCT c.state_key - FROM current_state_events AS c - INNER JOIN events AS e USING(room_id) - WHERE room_id = ? - AND c.membership = 'join' - AND e.type = 'm.room.message' - OR e.type = 'm.room.encrypted' - AND c.state_key = e.sender; + UPDATE room_memberships + SET participant = True + FROM ( + SELECT DISTINCT c.state_key, e.room_id + FROM current_state_events AS c + INNER JOIN events AS e ON c.room_id = e.room_id + WHERE c.membership = 'join' + AND c.state_key = e.sender + AND ( + e.type = 'm.room.message' + OR e.type = 'm.room.encrypted' + ) + ) AS subquery + WHERE room_memberships.user_id = subquery.state_key + AND room_memberships.room_id = subquery.room_id + AND room_memberships.event_stream_ordering <= ?; """ - txn.execute(sql, (current_room_id,)) - res = txn.fetchall() + txn.execute(sql, (stream_token,)) - if res: - participants = [user[0] for user in res] - for participant in participants: - self.db_pool.simple_update_txn( - txn, - table="room_memberships", - keyvalues={"user_id": participant, "room_id": current_room_id}, - updatevalues={"participant": True}, - ) + if stream_token is None: + stream_token = await self.db_pool.runInteraction( + "_get_max_stream_token", _get_max_stream_token_txn + ) - current_room_id = await self.db_pool.runInteraction( - "_get_current_room_txn", _get_current_room_txn, last_room_id - ) - if not current_room_id: + if stream_token <= 0: await self.db_pool.updates._end_background_update( "populate_participant_bg_update" ) - return 1 + return 1000 + logger.info(f"stream token is {stream_token}") await self.db_pool.runInteraction( - "_background_populate_participant_per_room_txn", - _background_populate_participant_per_room_txn, - current_room_id, + "_background_populate_participant_txn", + _background_populate_participant_txn, + stream_token, ) - progress["last_room_id"] = current_room_id + progress["last_stream_token"] = stream_token - 1000 await self.db_pool.runInteraction( "populate_participant_bg_update", self.db_pool.updates._background_update_progress_txn, "populate_participant_bg_update", progress, ) - return 1 + return 1000 async def _background_add_membership_profile( self, progress: JsonDict, batch_size: int diff --git a/synapse/storage/schema/main/delta/89/01_add_column_participant_room_memberships_table.sql b/synapse/storage/schema/main/delta/89/01_add_column_participant_room_memberships_table.sql index 3d8ee437137..089443c820e 100644 --- a/synapse/storage/schema/main/delta/89/01_add_column_participant_room_memberships_table.sql +++ b/synapse/storage/schema/main/delta/89/01_add_column_participant_room_memberships_table.sql @@ -12,7 +12,7 @@ -- . -- Add a column `participant` to `room_memberships` table to track whether a room member has sent --- a `m.room.message` event into a room they are a member of +-- a `m.room.message` or `m.room.encrypted` event into a room they are a member of ALTER TABLE room_memberships ADD COLUMN participant BOOLEAN DEFAULT FALSE; -- Add a background update to populate `participant` column From 4e8dbd60071b6f69adba7fe394838a8668bdfbbe Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Wed, 29 Jan 2025 16:58:56 -0800 Subject: [PATCH 10/24] lint + tests --- synapse/handlers/message.py | 2 +- synapse/storage/databases/main/roommember.py | 5 +++-- tests/rest/client/test_rooms.py | 8 ++++---- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index 0a1ecdf5616..b4cee395f0b 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -1441,7 +1441,7 @@ async def handle_new_client_event( return prev_event if event.type == "m.room.message" or event.type == "m.room.encrypted": - await self.store.set_room_participation(event.room_id, event.user_id) + await self.store.set_room_participation(event.user_id, event.room_id) if event.internal_metadata.is_out_of_band_membership(): # the only sort of out-of-band-membership events we expect to see here are diff --git a/synapse/storage/databases/main/roommember.py b/synapse/storage/databases/main/roommember.py index 7ab879fc134..cd27474dc71 100644 --- a/synapse/storage/databases/main/roommember.py +++ b/synapse/storage/databases/main/roommember.py @@ -1705,7 +1705,7 @@ async def _populate_participant(self, progress: JsonDict, batch_size: int) -> in """ stream_token = progress.get("last_stream_token", None) - def _get_max_stream_token_txn(txn: LoggingTransaction) -> Optional[str]: + def _get_max_stream_token_txn(txn: LoggingTransaction) -> Optional[int]: sql = """ SELECT event_stream_ordering from room_memberships ORDER BY event_stream_ordering DESC @@ -1713,7 +1713,8 @@ def _get_max_stream_token_txn(txn: LoggingTransaction) -> Optional[str]: """ txn.execute(sql) res = txn.fetchone() - assert res is not None + if not res: + return 0 return res[0] def _background_populate_participant_txn( diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py index ba0ff4d0451..4230bb58069 100644 --- a/tests/rest/client/test_rooms.py +++ b/tests/rest/client/test_rooms.py @@ -4102,7 +4102,7 @@ def test_sending_message_records_participation(self) -> None: # user has not sent any messages, so should not be a participant participant = self.get_success( - self.store.get_room_participation(self.room1, self.user2) + self.store.get_room_participation(self.user2, self.room1) ) self.assertFalse(participant) @@ -4117,7 +4117,7 @@ def test_sending_message_records_participation(self) -> None: tok=self.tok2, ) participant = self.get_success( - self.store.get_room_participation(self.room1, self.user2) + self.store.get_room_participation(self.user2, self.room1) ) self.assertTrue(participant) @@ -4130,7 +4130,7 @@ def test_sending_encrypted_event_records_participation(self) -> None: # user has not sent any messages, so should not be a participant participant = self.get_success( - self.store.get_room_participation(self.room1, self.user2) + self.store.get_room_participation(self.user2, self.room1) ) self.assertFalse(participant) @@ -4148,6 +4148,6 @@ def test_sending_encrypted_event_records_participation(self) -> None: tok=self.tok2, ) participant = self.get_success( - self.store.get_room_participation(self.room1, self.user2) + self.store.get_room_participation(self.user2, self.room1) ) self.assertTrue(participant) From bc8f3297a0035e717e743f79e2c684a12e2745af Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Wed, 29 Jan 2025 17:44:45 -0800 Subject: [PATCH 11/24] fix sql syntax for postgres --- synapse/storage/databases/main/roommember.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/synapse/storage/databases/main/roommember.py b/synapse/storage/databases/main/roommember.py index cd27474dc71..f98e534e283 100644 --- a/synapse/storage/databases/main/roommember.py +++ b/synapse/storage/databases/main/roommember.py @@ -1621,10 +1621,14 @@ def _set_room_participation_txn( sql = """ UPDATE room_memberships SET participant = True - WHERE user_id = ? - AND room_id = ? - ORDER BY event_stream_ordering DESC - LIMIT 1 + WHERE (user_id, room_id) IN ( + SELECT user_id, room_id + FROM room_memberships + WHERE user_id = ? + AND room_id = ? + ORDER BY event_stream_ordering DESC + LIMIT 1 + ) """ txn.execute(sql, (user_id, room_id)) From 1539eb64a99b82c886b07f485bdc84b5bd0191c5 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Wed, 29 Jan 2025 19:11:56 -0800 Subject: [PATCH 12/24] fix lint --- synapse/storage/databases/main/roommember.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/storage/databases/main/roommember.py b/synapse/storage/databases/main/roommember.py index f98e534e283..1364fb4caef 100644 --- a/synapse/storage/databases/main/roommember.py +++ b/synapse/storage/databases/main/roommember.py @@ -1641,8 +1641,8 @@ async def get_room_participation(self, user_id: str, room_id: str) -> bool: Check whether a user is listed as a participant in a room Args: - room_id: ID of the room to check in user_id: user ID of the user + room_id: ID of the room to check in """ def _get_room_participation_txn( From 8b958065caccfe649f5b987f89230915f251044b Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Wed, 29 Jan 2025 20:24:26 -0800 Subject: [PATCH 13/24] never return none --- synapse/storage/databases/main/roommember.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/storage/databases/main/roommember.py b/synapse/storage/databases/main/roommember.py index 1364fb4caef..eacc0fb81fd 100644 --- a/synapse/storage/databases/main/roommember.py +++ b/synapse/storage/databases/main/roommember.py @@ -1717,7 +1717,7 @@ def _get_max_stream_token_txn(txn: LoggingTransaction) -> Optional[int]: """ txn.execute(sql) res = txn.fetchone() - if not res: + if not res[0]: return 0 return res[0] From 3b9745dad0c57618aa956addf52c85f7cd9ecbeb Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Wed, 29 Jan 2025 20:40:51 -0800 Subject: [PATCH 14/24] lint --- synapse/storage/databases/main/roommember.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/storage/databases/main/roommember.py b/synapse/storage/databases/main/roommember.py index eacc0fb81fd..9b84894826a 100644 --- a/synapse/storage/databases/main/roommember.py +++ b/synapse/storage/databases/main/roommember.py @@ -1709,7 +1709,7 @@ async def _populate_participant(self, progress: JsonDict, batch_size: int) -> in """ stream_token = progress.get("last_stream_token", None) - def _get_max_stream_token_txn(txn: LoggingTransaction) -> Optional[int]: + def _get_max_stream_token_txn(txn: LoggingTransaction) -> int: sql = """ SELECT event_stream_ordering from room_memberships ORDER BY event_stream_ordering DESC @@ -1717,7 +1717,7 @@ def _get_max_stream_token_txn(txn: LoggingTransaction) -> Optional[int]: """ txn.execute(sql) res = txn.fetchone() - if not res[0]: + if not res or not res[0]: return 0 return res[0] From 72f283f2da616c3f504f609b8d0cee79b95b8ef7 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Wed, 12 Feb 2025 13:07:43 -0800 Subject: [PATCH 15/24] requested changes --- synapse/storage/databases/main/roommember.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/synapse/storage/databases/main/roommember.py b/synapse/storage/databases/main/roommember.py index 9b84894826a..3dd321e0fda 100644 --- a/synapse/storage/databases/main/roommember.py +++ b/synapse/storage/databases/main/roommember.py @@ -79,6 +79,7 @@ _MEMBERSHIP_PROFILE_UPDATE_NAME = "room_membership_profile_update" _CURRENT_STATE_MEMBERSHIP_UPDATE_NAME = "current_state_events_membership" +_POPULATE_PARTICIPANT_BG_UPDATE_BATCH_SIZE = 1000 @attr.s(frozen=True, slots=True, auto_attribs=True) @@ -1620,7 +1621,7 @@ def _set_room_participation_txn( ) -> None: sql = """ UPDATE room_memberships - SET participant = True + SET participant = true WHERE (user_id, room_id) IN ( SELECT user_id, room_id FROM room_memberships @@ -1740,7 +1741,8 @@ def _background_populate_participant_txn( ) AS subquery WHERE room_memberships.user_id = subquery.state_key AND room_memberships.room_id = subquery.room_id - AND room_memberships.event_stream_ordering <= ?; + AND room_memberships.event_stream_ordering <= ? + AND room_memberships.event_stream_ordering >= 0; """ txn.execute(sql, (stream_token,)) @@ -1754,23 +1756,22 @@ def _background_populate_participant_txn( await self.db_pool.updates._end_background_update( "populate_participant_bg_update" ) - return 1000 + return _POPULATE_PARTICIPANT_BG_UPDATE_BATCH_SIZE - logger.info(f"stream token is {stream_token}") await self.db_pool.runInteraction( "_background_populate_participant_txn", _background_populate_participant_txn, stream_token, ) - progress["last_stream_token"] = stream_token - 1000 + progress["last_stream_token"] = stream_token - _POPULATE_PARTICIPANT_BG_UPDATE_BATCH_SIZE await self.db_pool.runInteraction( "populate_participant_bg_update", self.db_pool.updates._background_update_progress_txn, "populate_participant_bg_update", progress, ) - return 1000 + return _POPULATE_PARTICIPANT_BG_UPDATE_BATCH_SIZE async def _background_add_membership_profile( self, progress: JsonDict, batch_size: int From 3db14a7b4fa2f17fccdffa3a430713a113a5566b Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Wed, 12 Feb 2025 13:20:37 -0800 Subject: [PATCH 16/24] fix merge conflict with develop --- synapse/storage/schema/__init__.py | 5 ++++- .../01_add_column_participant_room_memberships_table.sql | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) rename synapse/storage/schema/main/delta/{89 => 90}/01_add_column_participant_room_memberships_table.sql (94%) diff --git a/synapse/storage/schema/__init__.py b/synapse/storage/schema/__init__.py index 49e648a92fd..69da244c91f 100644 --- a/synapse/storage/schema/__init__.py +++ b/synapse/storage/schema/__init__.py @@ -19,7 +19,7 @@ # # -SCHEMA_VERSION = 89 # remember to update the list below when updating +SCHEMA_VERSION = 90 # remember to update the list below when updating """Represents the expectations made by the codebase about the database schema This should be incremented whenever the codebase changes its requirements on the @@ -158,6 +158,9 @@ Changes in SCHEMA_VERSION = 89 - Add `state_groups_pending_deletion` and `state_groups_persisting` tables. + +Changes in SCHEMA_VERSION = 90 + - Add a column `participant` to `room_memberships` table """ diff --git a/synapse/storage/schema/main/delta/89/01_add_column_participant_room_memberships_table.sql b/synapse/storage/schema/main/delta/90/01_add_column_participant_room_memberships_table.sql similarity index 94% rename from synapse/storage/schema/main/delta/89/01_add_column_participant_room_memberships_table.sql rename to synapse/storage/schema/main/delta/90/01_add_column_participant_room_memberships_table.sql index 089443c820e..672be1031ef 100644 --- a/synapse/storage/schema/main/delta/89/01_add_column_participant_room_memberships_table.sql +++ b/synapse/storage/schema/main/delta/90/01_add_column_participant_room_memberships_table.sql @@ -17,4 +17,4 @@ ALTER TABLE room_memberships ADD COLUMN participant BOOLEAN DEFAULT FALSE; -- Add a background update to populate `participant` column INSERT INTO background_updates (ordering, update_name, progress_json) VALUES - (8901, 'populate_participant_bg_update', '{}'); \ No newline at end of file + (9001, 'populate_participant_bg_update', '{}'); \ No newline at end of file From db6cbaa6c49d9570e08ab7cd985ffcb877977387 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Wed, 12 Feb 2025 14:06:04 -0800 Subject: [PATCH 17/24] lint --- synapse/storage/databases/main/roommember.py | 4 +++- synapse/storage/schema/__init__.py | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/synapse/storage/databases/main/roommember.py b/synapse/storage/databases/main/roommember.py index 3dd321e0fda..9b9e032ba75 100644 --- a/synapse/storage/databases/main/roommember.py +++ b/synapse/storage/databases/main/roommember.py @@ -1764,7 +1764,9 @@ def _background_populate_participant_txn( stream_token, ) - progress["last_stream_token"] = stream_token - _POPULATE_PARTICIPANT_BG_UPDATE_BATCH_SIZE + progress["last_stream_token"] = ( + stream_token - _POPULATE_PARTICIPANT_BG_UPDATE_BATCH_SIZE + ) await self.db_pool.runInteraction( "populate_participant_bg_update", self.db_pool.updates._background_update_progress_txn, diff --git a/synapse/storage/schema/__init__.py b/synapse/storage/schema/__init__.py index 69da244c91f..f87b1a4a0a3 100644 --- a/synapse/storage/schema/__init__.py +++ b/synapse/storage/schema/__init__.py @@ -158,7 +158,7 @@ Changes in SCHEMA_VERSION = 89 - Add `state_groups_pending_deletion` and `state_groups_persisting` tables. - + Changes in SCHEMA_VERSION = 90 - Add a column `participant` to `room_memberships` table """ From dd0a538d8de9f0621be73d4fd3e083ef46f305d7 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Wed, 5 Mar 2025 11:48:51 -0800 Subject: [PATCH 18/24] fix merge conflit error --- tests/rest/client/test_rooms.py | 94 ++++++++++++++++++++++++++++++++- 1 file changed, 92 insertions(+), 2 deletions(-) diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py index 2b45e78b7b4..f409919bfed 100644 --- a/tests/rest/client/test_rooms.py +++ b/tests/rest/client/test_rooms.py @@ -1372,6 +1372,23 @@ def test_suspended_user_cannot_invite_to_room(self) -> None: ) self.assertEqual(channel.json_body["errcode"], "M_USER_SUSPENDED") + def test_suspended_user_can_leave_room(self) -> None: + channel = self.make_request( + "POST", f"/join/{self.room1}", access_token=self.tok1 + ) + self.assertEqual(channel.code, 200) + + # set the user as suspended + self.get_success(self.store.set_user_suspended_status(self.user1, True)) + + # leave room + channel = self.make_request( + "POST", + f"/rooms/{self.room1}/leave", + access_token=self.tok1, + ) + self.assertEqual(channel.code, 200) + class RoomAppserviceTsParamTestCase(unittest.HomeserverTestCase): servlets = [ @@ -2382,6 +2399,41 @@ def test_send_delayed_state_event(self) -> None: ) self.assertEqual(HTTPStatus.OK, channel.code, channel.result) + @unittest.override_config( + { + "max_event_delay_duration": "24h", + "rc_message": {"per_second": 1, "burst_count": 2}, + } + ) + def test_add_delayed_event_ratelimit(self) -> None: + """Test that requests to schedule new delayed events are ratelimited by a RateLimiter, + which ratelimits them correctly, including by not limiting when the requester is + exempt from ratelimiting. + """ + + # Test that new delayed events are correctly ratelimited. + args = ( + "POST", + ( + "rooms/%s/send/m.room.message?org.matrix.msc4140.delay=2000" + % self.room_id + ).encode("ascii"), + {"body": "test", "msgtype": "m.text"}, + ) + channel = self.make_request(*args) + self.assertEqual(HTTPStatus.OK, channel.code, channel.result) + channel = self.make_request(*args) + self.assertEqual(HTTPStatus.TOO_MANY_REQUESTS, channel.code, channel.result) + + # Add the current user to the ratelimit overrides, allowing them no ratelimiting. + self.get_success( + self.hs.get_datastores().main.set_ratelimit_for_user(self.user_id, 0, 0) + ) + + # Test that the new delayed events aren't ratelimited anymore. + channel = self.make_request(*args) + self.assertEqual(HTTPStatus.OK, channel.code, channel.result) + class RoomSearchTestCase(unittest.HomeserverTestCase): servlets = [ @@ -2549,6 +2601,11 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: tok=self.token, ) + def default_config(self) -> JsonDict: + config = default_config("test") + config["room_list_publication_rules"] = [{"action": "allow"}] + return config + def make_public_rooms_request( self, room_types: Optional[List[Union[str, None]]], @@ -3990,10 +4047,25 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: self.user2 = self.register_user("teresa", "hackme") self.tok2 = self.login("teresa", "hackme") - self.room1 = self.helper.create_room_as(room_creator=self.user1, tok=self.tok1) + self.admin = self.register_user("admin", "pass", True) + self.admin_tok = self.login("admin", "pass") + + self.room1 = self.helper.create_room_as( + room_creator=self.user1, tok=self.tok1, room_version="11" + ) self.store = hs.get_datastores().main - def test_suspended_user_cannot_send_message_to_room(self) -> None: + self.room2 = self.helper.create_room_as( + room_creator=self.user1, is_public=False, tok=self.tok1 + ) + self.helper.send_state( + self.room2, + EventTypes.RoomEncryption, + {EventContentFields.ENCRYPTION_ALGORITHM: "m.megolm.v1.aes-sha2"}, + tok=self.tok1, + ) + + def test_suspended_user_cannot_send_message_to_public_room(self) -> None: # set the user as suspended self.get_success(self.store.set_user_suspended_status(self.user1, True)) @@ -4005,6 +4077,24 @@ def test_suspended_user_cannot_send_message_to_room(self) -> None: ) self.assertEqual(channel.json_body["errcode"], "M_USER_SUSPENDED") + def test_suspended_user_cannot_send_message_to_encrypted_room(self) -> None: + channel = self.make_request( + "PUT", + f"/_synapse/admin/v1/suspend/{self.user1}", + {"suspend": True}, + access_token=self.admin_tok, + ) + self.assertEqual(channel.code, 200) + self.assertEqual(channel.json_body, {f"user_{self.user1}_suspended": True}) + + channel = self.make_request( + "PUT", + f"/rooms/{self.room2}/send/m.room.encrypted/1", + access_token=self.tok1, + content={}, + ) + self.assertEqual(channel.json_body["errcode"], "M_USER_SUSPENDED") + def test_suspended_user_cannot_change_profile_data(self) -> None: # set the user as suspended self.get_success(self.store.set_user_suspended_status(self.user1, True)) From f934de4aaad936449c9a2e707e8d01277c86fffb Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Mon, 17 Mar 2025 12:22:16 -0700 Subject: [PATCH 19/24] requested changes --- synapse/storage/databases/main/roommember.py | 19 ++++++++++++++----- synapse/storage/schema/__init__.py | 1 - 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/synapse/storage/databases/main/roommember.py b/synapse/storage/databases/main/roommember.py index 9b9e032ba75..a0a6dcd04e7 100644 --- a/synapse/storage/databases/main/roommember.py +++ b/synapse/storage/databases/main/roommember.py @@ -1705,8 +1705,17 @@ async def _populate_participant(self, progress: JsonDict, batch_size: int) -> in """ Background update to populate column `participant` on `room_memberships` table - A 'participant' is someone who is currently joined to a room and has sent at least + A 'participant' is someone who is currently joined to a room and has sent at least one `m.room.message` or `m.room.encrypted` event. + + This background update will set the `participant` column across all rows in + `room_memberships` based on the user's *current* join status, and if + they've *ever* sent a message or encrypted event. Therefore one should + never assume the `participant` column's value is based solely on whether + the user participated in a previous "session" (where a "session" is defined + as a period between the user joining and leaving). See + https://github.com/element-hq/synapse/pull/18068#discussion_r1931070291 + for further detail. """ stream_token = progress.get("last_stream_token", None) @@ -1742,17 +1751,17 @@ def _background_populate_participant_txn( WHERE room_memberships.user_id = subquery.state_key AND room_memberships.room_id = subquery.room_id AND room_memberships.event_stream_ordering <= ? - AND room_memberships.event_stream_ordering >= 0; + AND room_memberships.event_stream_ordering > ?; """ - - txn.execute(sql, (stream_token,)) + batch = int(stream_token) - _POPULATE_PARTICIPANT_BG_UPDATE_BATCH_SIZE + txn.execute(sql, (stream_token, batch)) if stream_token is None: stream_token = await self.db_pool.runInteraction( "_get_max_stream_token", _get_max_stream_token_txn ) - if stream_token <= 0: + if stream_token < 0: await self.db_pool.updates._end_background_update( "populate_participant_bg_update" ) diff --git a/synapse/storage/schema/__init__.py b/synapse/storage/schema/__init__.py index 7cd8ec3bdc5..f87b1a4a0a3 100644 --- a/synapse/storage/schema/__init__.py +++ b/synapse/storage/schema/__init__.py @@ -158,7 +158,6 @@ Changes in SCHEMA_VERSION = 89 - Add `state_groups_pending_deletion` and `state_groups_persisting` tables. - - Add background update to delete unreferenced state groups. Changes in SCHEMA_VERSION = 90 - Add a column `participant` to `room_memberships` table From c7dc36cf587c9f40e519cc0e0d61d586fd213f8b Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Mon, 17 Mar 2025 12:52:26 -0700 Subject: [PATCH 20/24] extra tests --- tests/rest/client/test_rooms.py | 79 +++++++++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py index f409919bfed..968e9567305 100644 --- a/tests/rest/client/test_rooms.py +++ b/tests/rest/client/test_rooms.py @@ -4289,3 +4289,82 @@ def test_sending_encrypted_event_records_participation(self) -> None: self.store.get_room_participation(self.user2, self.room1) ) self.assertTrue(participant) + + def test_sending_message_and_leaving_does_not_record_participation(self) -> None: + """ + Test that sending an m.room.message event into a room and then leaving + causes the user to no longer be marked as a participant in that room + """ + self.helper.join(self.room1, self.user2, tok=self.tok2) + + # user has not sent any messages, so should not be a participant + participant = self.get_success( + self.store.get_room_participation(self.user2, self.room1) + ) + self.assertFalse(participant) + + # sending a message should now mark user as participant + self.helper.send_event( + self.room1, + "m.room.message", + content={ + "msgtype": "m.text", + "body": "I am engaging in this room", + }, + tok=self.tok2, + ) + participant = self.get_success( + self.store.get_room_participation(self.user2, self.room1) + ) + self.assertTrue(participant) + + # leave the room + self.helper.leave(self.room1, self.user2, tok=self.tok2) + + # user should no longer be considered a participant + participant = self.get_success( + self.store.get_room_participation(self.user2, self.room1) + ) + self.assertFalse(participant) + + def test_sending_encrypted_event_and_leaving_does_not_record_participation( + self, + ) -> None: + """ + Test that sending an m.room.encrypted event into a room and then leaving causes + the user to no longer be marked as a participant in that room + """ + self.helper.join(self.room1, self.user2, tok=self.tok2) + + # user has not sent any messages, so should not be a participant + participant = self.get_success( + self.store.get_room_participation(self.user2, self.room1) + ) + self.assertFalse(participant) + + # sending an encrypted event should now mark user as participant + self.helper.send_event( + self.room1, + "m.room.encrypted", + content={ + "algorithm": "m.megolm.v1.aes-sha2", + "ciphertext": "AwgAEnACgAkLmt6qF84IK++J7UDH2Za1YVchHyprqTqsg...", + "device_id": "RJYKSTBOIE", + "sender_key": "IlRMeOPX2e0MurIyfWEucYBRVOEEUMrOHqn/8mLqMjA", + "session_id": "X3lUlvLELLYxeTx4yOVu6UDpasGEVO0Jbu+QFnm0cKQ", + }, + tok=self.tok2, + ) + participant = self.get_success( + self.store.get_room_participation(self.user2, self.room1) + ) + self.assertTrue(participant) + + # leave the room + self.helper.leave(self.room1, self.user2, tok=self.tok2) + + # user should no longer be considered a participant + participant = self.get_success( + self.store.get_room_participation(self.user2, self.room1) + ) + self.assertFalse(participant) From af81dd65b249368386ffb2f6dc2e563e05d04aa5 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Tue, 18 Mar 2025 10:31:41 +0000 Subject: [PATCH 21/24] Convert tests to use parameterized To allow to easily adding more test cases. --- tests/rest/client/test_rooms.py | 157 ++++++++++++++------------------ 1 file changed, 68 insertions(+), 89 deletions(-) diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py index 968e9567305..1127c3daad0 100644 --- a/tests/rest/client/test_rooms.py +++ b/tests/rest/client/test_rooms.py @@ -4231,69 +4231,42 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: ) self.store = hs.get_datastores().main - def test_sending_message_records_participation(self) -> None: - """ - Test that sending an m.room.message event into a room causes the user to - be marked as a participant in that room - """ - self.helper.join(self.room1, self.user2, tok=self.tok2) - - # user has not sent any messages, so should not be a participant - participant = self.get_success( - self.store.get_room_participation(self.user2, self.room1) - ) - self.assertFalse(participant) - - # sending a message should now mark user as participant - self.helper.send_event( - self.room1, - "m.room.message", - content={ - "msgtype": "m.text", - "body": "I am engaging in this room", - }, - tok=self.tok2, - ) - participant = self.get_success( - self.store.get_room_participation(self.user2, self.room1) - ) - self.assertTrue(participant) - - def test_sending_encrypted_event_records_participation(self) -> None: - """ - Test that sending an m.room.encrypted event into a room causes the user to - be marked as a participant in that room - """ - self.helper.join(self.room1, self.user2, tok=self.tok2) - - # user has not sent any messages, so should not be a participant - participant = self.get_success( - self.store.get_room_participation(self.user2, self.room1) - ) - self.assertFalse(participant) - - # sending an encrypted event should now mark user as participant - self.helper.send_event( - self.room1, - "m.room.encrypted", - content={ - "algorithm": "m.megolm.v1.aes-sha2", - "ciphertext": "AwgAEnACgAkLmt6qF84IK++J7UDH2Za1YVchHyprqTqsg...", - "device_id": "RJYKSTBOIE", - "sender_key": "IlRMeOPX2e0MurIyfWEucYBRVOEEUMrOHqn/8mLqMjA", - "session_id": "X3lUlvLELLYxeTx4yOVu6UDpasGEVO0Jbu+QFnm0cKQ", - }, - tok=self.tok2, - ) - participant = self.get_success( - self.store.get_room_participation(self.user2, self.room1) - ) - self.assertTrue(participant) - - def test_sending_message_and_leaving_does_not_record_participation(self) -> None: + @parameterized.expand( + [ + # Allow + param( + is_state=False, + event_type="m.room.message", + event_content={ + "msgtype": "m.text", + "body": "I am engaging in this room", + }, + record_participation=True, + ), + param( + is_state=False, + event_type="m.room.encrypted", + event_content={ + "algorithm": "m.megolm.v1.aes-sha2", + "ciphertext": "AwgAEnACgAkLmt6qF84IK++J7UDH2Za1YVchHyprqTqsg...", + "device_id": "RJYKSTBOIE", + "sender_key": "IlRMeOPX2e0MurIyfWEucYBRVOEEUMrOHqn/8mLqMjA", + "session_id": "X3lUlvLELLYxeTx4yOVu6UDpasGEVO0Jbu+QFnm0cKQ", + }, + record_participation=True, + ), + ] + ) + def test_sending_message_records_participation( + self, + is_state: bool, + event_type: str, + event_content: JsonDict, + record_participation: bool, + ) -> None: """ - Test that sending an m.room.message event into a room and then leaving - causes the user to no longer be marked as a participant in that room + Test that sending an various events into a room causes the user to + appropriately marked or not marked as a participant in that room. """ self.helper.join(self.room1, self.user2, tok=self.tok2) @@ -4306,33 +4279,45 @@ def test_sending_message_and_leaving_does_not_record_participation(self) -> None # sending a message should now mark user as participant self.helper.send_event( self.room1, - "m.room.message", - content={ - "msgtype": "m.text", - "body": "I am engaging in this room", - }, + event_type, + content=event_content, tok=self.tok2, ) participant = self.get_success( self.store.get_room_participation(self.user2, self.room1) ) - self.assertTrue(participant) - - # leave the room - self.helper.leave(self.room1, self.user2, tok=self.tok2) - - # user should no longer be considered a participant - participant = self.get_success( - self.store.get_room_participation(self.user2, self.room1) - ) - self.assertFalse(participant) + self.assertEqual(participant, record_participation) - def test_sending_encrypted_event_and_leaving_does_not_record_participation( + @parameterized.expand( + [ + param( + event_type="m.room.message", + event_content={ + "msgtype": "m.text", + "body": "I am engaging in this room", + }, + ), + param( + event_type="m.room.encrypted", + event_content={ + "algorithm": "m.megolm.v1.aes-sha2", + "ciphertext": "AwgAEnACgAkLmt6qF84IK++J7UDH2Za1YVchHyprqTqsg...", + "device_id": "RJYKSTBOIE", + "sender_key": "IlRMeOPX2e0MurIyfWEucYBRVOEEUMrOHqn/8mLqMjA", + "session_id": "X3lUlvLELLYxeTx4yOVu6UDpasGEVO0Jbu+QFnm0cKQ", + }, + ), + ] + ) + def test_sending_event_and_leaving_does_not_record_participation( self, + event_type: str, + event_content: JsonDict, ) -> None: """ - Test that sending an m.room.encrypted event into a room and then leaving causes - the user to no longer be marked as a participant in that room + Test that sending an event into a room that should mark a user as a + participant, but then leaving the room, results in the user no longer + be marked as a participant in that room. """ self.helper.join(self.room1, self.user2, tok=self.tok2) @@ -4342,17 +4327,11 @@ def test_sending_encrypted_event_and_leaving_does_not_record_participation( ) self.assertFalse(participant) - # sending an encrypted event should now mark user as participant + # sending a message should now mark user as participant self.helper.send_event( self.room1, - "m.room.encrypted", - content={ - "algorithm": "m.megolm.v1.aes-sha2", - "ciphertext": "AwgAEnACgAkLmt6qF84IK++J7UDH2Za1YVchHyprqTqsg...", - "device_id": "RJYKSTBOIE", - "sender_key": "IlRMeOPX2e0MurIyfWEucYBRVOEEUMrOHqn/8mLqMjA", - "session_id": "X3lUlvLELLYxeTx4yOVu6UDpasGEVO0Jbu+QFnm0cKQ", - }, + event_type, + content=event_content, tok=self.tok2, ) participant = self.get_success( From 5e105f432c4fd7f1b564b9d8b94859d3d5db64e8 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Tue, 18 Mar 2025 10:41:10 +0000 Subject: [PATCH 22/24] Allow testing state events Allow user2 to send state events into the room. --- tests/rest/client/test_rooms.py | 40 ++++++++++++++++++++++++--------- 1 file changed, 29 insertions(+), 11 deletions(-) diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py index 1127c3daad0..a6fb08b0a4c 100644 --- a/tests/rest/client/test_rooms.py +++ b/tests/rest/client/test_rooms.py @@ -4225,15 +4225,21 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: self.user2 = self.register_user("teresa", "hackme") self.tok2 = self.login("teresa", "hackme") - self.room1 = self.helper.create_room_as(room_creator=self.user1, tok=self.tok1) - self.room2 = self.helper.create_room_as( - self.user1, is_public=False, tok=self.tok1 + self.room1 = self.helper.create_room_as( + room_creator=self.user1, + tok=self.tok1, + # Allow user2 to send state events into the room. + extra_content={ + "power_level_content_override": { + "state_default": 0, + }, + }, ) self.store = hs.get_datastores().main @parameterized.expand( [ - # Allow + # Should record participation. param( is_state=False, event_type="m.room.message", @@ -4276,13 +4282,25 @@ def test_sending_message_records_participation( ) self.assertFalse(participant) - # sending a message should now mark user as participant - self.helper.send_event( - self.room1, - event_type, - content=event_content, - tok=self.tok2, - ) + # send an event into the room + if is_state: + # send a state event + self.helper.send_state( + self.room1, + event_type, + body=event_content, + tok=self.tok2, + ) + else: + # send a non-state event + self.helper.send_event( + self.room1, + event_type, + content=event_content, + tok=self.tok2, + ) + + # check whether the user has been marked as a participant participant = self.get_success( self.store.get_room_participation(self.user2, self.room1) ) From 109a326c753a474ce0b588f49e00fae1021f86c2 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Tue, 18 Mar 2025 10:41:50 +0000 Subject: [PATCH 23/24] Don't mark user as participant when sending state events Even if the type of the state event tries to replicate that of a non-state message or encrypted event. --- synapse/handlers/message.py | 5 ++++- tests/rest/client/test_rooms.py | 26 ++++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index 7cd0e5966e9..52c61cfa542 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -1462,7 +1462,10 @@ async def handle_new_client_event( ) return prev_event - if event.type == "m.room.message" or event.type == "m.room.encrypted": + if not event.is_state() and event.type in [ + EventTypes.Message, + EventTypes.Encrypted, + ]: await self.store.set_room_participation(event.user_id, event.room_id) if event.internal_metadata.is_out_of_band_membership(): diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py index a6fb08b0a4c..69150359724 100644 --- a/tests/rest/client/test_rooms.py +++ b/tests/rest/client/test_rooms.py @@ -4261,6 +4261,32 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: }, record_participation=True, ), + # Should not record participation. + # An invalid **state event** with type `m.room.message` + param( + is_state=True, + event_type="m.room.message", + event_content={ + "msgtype": "m.text", + "body": "I am engaging in this room", + }, + record_participation=False, + ), + # An invalid **state event** with type `m.room.encrypted` + # Note: this may become valid in the future with encrypted state, though we + # still may not want to consider it grounds for marking a user as participating. + param( + is_state=True, + event_type="m.room.encrypted", + event_content={ + "algorithm": "m.megolm.v1.aes-sha2", + "ciphertext": "AwgAEnACgAkLmt6qF84IK++J7UDH2Za1YVchHyprqTqsg...", + "device_id": "RJYKSTBOIE", + "sender_key": "IlRMeOPX2e0MurIyfWEucYBRVOEEUMrOHqn/8mLqMjA", + "session_id": "X3lUlvLELLYxeTx4yOVu6UDpasGEVO0Jbu+QFnm0cKQ", + }, + record_participation=False, + ), ] ) def test_sending_message_records_participation( From d85541bd9a9491aac3056dafe73b2a70f96656bf Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Tue, 18 Mar 2025 10:46:38 +0000 Subject: [PATCH 24/24] Add test case for non-message/encrypted event type Users can send stickers and not be considered a participant. --- tests/rest/client/test_rooms.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py index 69150359724..6c93ead3b89 100644 --- a/tests/rest/client/test_rooms.py +++ b/tests/rest/client/test_rooms.py @@ -4262,6 +4262,16 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: record_participation=True, ), # Should not record participation. + param( + is_state=False, + event_type="m.sticker", + event_content={ + "body": "My great sticker", + "info": {}, + "url": "mxc://unused/mxcurl", + }, + record_participation=False, + ), # An invalid **state event** with type `m.room.message` param( is_state=True,