From 914076d5943df467e750b85dda9ce843325943f4 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 5 Apr 2022 12:02:55 -0400 Subject: [PATCH 1/4] Remove references to unstable thread relation. --- synapse/api/constants.py | 2 - synapse/events/utils.py | 2 - synapse/handlers/message.py | 5 +- synapse/storage/databases/main/events.py | 5 +- synapse/storage/databases/main/relations.py | 57 ++++++--------------- 5 files changed, 17 insertions(+), 54 deletions(-) diff --git a/synapse/api/constants.py b/synapse/api/constants.py index 92907415e651..0172eb60b8dc 100644 --- a/synapse/api/constants.py +++ b/synapse/api/constants.py @@ -179,8 +179,6 @@ class RelationTypes: REPLACE: Final = "m.replace" REFERENCE: Final = "m.reference" THREAD: Final = "m.thread" - # TODO Remove this in Synapse >= v1.57.0. - UNSTABLE_THREAD: Final = "io.element.thread" class LimitBlockingTypes: diff --git a/synapse/events/utils.py b/synapse/events/utils.py index 918e87ed9cf1..3203e11b087a 100644 --- a/synapse/events/utils.py +++ b/synapse/events/utils.py @@ -525,8 +525,6 @@ def _inject_bundled_aggregations( "current_user_participated": thread.current_user_participated, } serialized_aggregations[RelationTypes.THREAD] = thread_summary - if self._msc3440_enabled: - serialized_aggregations[RelationTypes.UNSTABLE_THREAD] = thread_summary # Include the bundled aggregations in the event. if serialized_aggregations: diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index 7db6905c6165..47a63005a984 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -1102,10 +1102,7 @@ async def _validate_event_relation(self, event: EventBase) -> None: raise SynapseError(400, "Can't send same reaction twice") # Don't attempt to start a thread if the parent event is a relation. - elif ( - relation_type == RelationTypes.THREAD - or relation_type == RelationTypes.UNSTABLE_THREAD - ): + elif relation_type == RelationTypes.THREAD: if await self.store.event_includes_relation(relates_to): raise SynapseError( 400, "Cannot start threads from an event with a relation" diff --git a/synapse/storage/databases/main/events.py b/synapse/storage/databases/main/events.py index 3fcd5f5b99de..e3be537cee35 100644 --- a/synapse/storage/databases/main/events.py +++ b/synapse/storage/databases/main/events.py @@ -1819,10 +1819,7 @@ def _handle_event_relations( if rel_type == RelationTypes.REPLACE: txn.call_after(self.store.get_applicable_edit.invalidate, (parent_id,)) - if ( - rel_type == RelationTypes.THREAD - or rel_type == RelationTypes.UNSTABLE_THREAD - ): + if rel_type == RelationTypes.THREAD: txn.call_after(self.store.get_thread_summary.invalidate, (parent_id,)) # It should be safe to only invalidate the cache if the user has not # previously participated in the thread, but that's difficult (and diff --git a/synapse/storage/databases/main/relations.py b/synapse/storage/databases/main/relations.py index db929ef523ee..43ca17232081 100644 --- a/synapse/storage/databases/main/relations.py +++ b/synapse/storage/databases/main/relations.py @@ -497,7 +497,7 @@ def _get_thread_summaries_txn( AND parent.room_id = child.room_id WHERE %s - AND %s + AND relation_type = ? ORDER BY parent.event_id, child.topological_ordering DESC, child.stream_ordering DESC """ else: @@ -512,22 +512,16 @@ def _get_thread_summaries_txn( AND parent.room_id = child.room_id WHERE %s - AND %s + AND relation_type = ? ORDER BY child.topological_ordering DESC, child.stream_ordering DESC """ clause, args = make_in_list_sql_clause( txn.database_engine, "relates_to_id", event_ids ) + args.append(RelationTypes.THREAD) - if self._msc3440_enabled: - relations_clause = "(relation_type = ? OR relation_type = ?)" - args.extend((RelationTypes.THREAD, RelationTypes.UNSTABLE_THREAD)) - else: - relations_clause = "relation_type = ?" - args.append(RelationTypes.THREAD) - - txn.execute(sql % (clause, relations_clause), args) + txn.execute(sql % (clause,), args) latest_event_ids = {} for parent_event_id, child_event_id in txn: # Only consider the latest threaded reply (by topological ordering). @@ -547,7 +541,7 @@ def _get_thread_summaries_txn( AND parent.room_id = child.room_id WHERE %s - AND %s + AND relation_type = ? GROUP BY parent.event_id """ @@ -556,15 +550,9 @@ def _get_thread_summaries_txn( clause, args = make_in_list_sql_clause( txn.database_engine, "relates_to_id", latest_event_ids.keys() ) + args.append(RelationTypes.THREAD) - if self._msc3440_enabled: - relations_clause = "(relation_type = ? OR relation_type = ?)" - args.extend((RelationTypes.THREAD, RelationTypes.UNSTABLE_THREAD)) - else: - relations_clause = "relation_type = ?" - args.append(RelationTypes.THREAD) - - txn.execute(sql % (clause, relations_clause), args) + txn.execute(sql % (clause,), args) counts = dict(cast(List[Tuple[str, int]], txn.fetchall())) return counts, latest_event_ids @@ -622,7 +610,7 @@ async def get_threaded_messages_per_user( parent.event_id = relates_to_id AND parent.room_id = child.room_id WHERE - %s + relation_type = ? AND %s AND %s GROUP BY parent.event_id, child.sender @@ -638,16 +626,9 @@ def _get_threaded_messages_per_user_txn( txn.database_engine, "relates_to_id", event_ids ) - if self._msc3440_enabled: - relations_clause = "(relation_type = ? OR relation_type = ?)" - relations_args = [RelationTypes.THREAD, RelationTypes.UNSTABLE_THREAD] - else: - relations_clause = "relation_type = ?" - relations_args = [RelationTypes.THREAD] - txn.execute( - sql % (users_sql, events_clause, relations_clause), - users_args + events_args + relations_args, + sql % (users_sql, events_clause), + [RelationTypes.THREAD] + users_args + events_args, ) return {(row[0], row[1]): row[2] for row in txn} @@ -677,7 +658,7 @@ async def get_threads_participated( user participated in that event's thread, otherwise false. """ - def _get_thread_summary_txn(txn: LoggingTransaction) -> Set[str]: + def _get_threads_participated_txn(txn: LoggingTransaction) -> Set[str]: # Fetch whether the requester has participated or not. sql = """ SELECT DISTINCT relates_to_id @@ -688,28 +669,20 @@ def _get_thread_summary_txn(txn: LoggingTransaction) -> Set[str]: AND parent.room_id = child.room_id WHERE %s - AND %s + AND relation_type = ? AND child.sender = ? """ clause, args = make_in_list_sql_clause( txn.database_engine, "relates_to_id", event_ids ) + args.extend([RelationTypes.THREAD, user_id]) - if self._msc3440_enabled: - relations_clause = "(relation_type = ? OR relation_type = ?)" - args.extend((RelationTypes.THREAD, RelationTypes.UNSTABLE_THREAD)) - else: - relations_clause = "relation_type = ?" - args.append(RelationTypes.THREAD) - - args.append(user_id) - - txn.execute(sql % (clause, relations_clause), args) + txn.execute(sql % (clause,), args) return {row[0] for row in txn.fetchall()} participated_threads = await self.db_pool.runInteraction( - "get_thread_summary", _get_thread_summary_txn + "get_threads_participated", _get_threads_participated_txn ) return {event_id: event_id in participated_threads for event_id in event_ids} From c4c2e2fa8390a3ee0cb4a1474cbf5997daa75a06 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 5 Apr 2022 12:04:33 -0400 Subject: [PATCH 2/4] Remove unstable identifiers for filtering parameters. --- synapse/api/filtering.py | 12 ------------ tests/api/test_filtering.py | 4 +--- 2 files changed, 1 insertion(+), 15 deletions(-) diff --git a/synapse/api/filtering.py b/synapse/api/filtering.py index 27e97d6f372d..4a808e33fee1 100644 --- a/synapse/api/filtering.py +++ b/synapse/api/filtering.py @@ -89,9 +89,7 @@ "org.matrix.not_labels": {"type": "array", "items": {"type": "string"}}, # MSC3440, filtering by event relations. "related_by_senders": {"type": "array", "items": {"type": "string"}}, - "io.element.relation_senders": {"type": "array", "items": {"type": "string"}}, "related_by_rel_types": {"type": "array", "items": {"type": "string"}}, - "io.element.relation_types": {"type": "array", "items": {"type": "string"}}, }, } @@ -323,16 +321,6 @@ def __init__(self, hs: "HomeServer", filter_json: JsonDict): self.related_by_senders = self.filter_json.get("related_by_senders", None) self.related_by_rel_types = self.filter_json.get("related_by_rel_types", None) - # Fallback to the unstable prefix if the stable version is not given. - if hs.config.experimental.msc3440_enabled: - self.related_by_senders = self.related_by_senders or self.filter_json.get( - "io.element.relation_senders", None - ) - self.related_by_rel_types = ( - self.related_by_rel_types - or self.filter_json.get("io.element.relation_types", None) - ) - def filters_all_types(self) -> bool: return "*" in self.not_types diff --git a/tests/api/test_filtering.py b/tests/api/test_filtering.py index 8c3354ce3c79..985d6e397d36 100644 --- a/tests/api/test_filtering.py +++ b/tests/api/test_filtering.py @@ -481,9 +481,7 @@ def test_filter_relations(self): # events). This is a bit cheeky, but tests the logic of _check_event_relations. # Filter for a particular sender. - definition = { - "io.element.relation_senders": ["@foo:bar"], - } + definition = {"related_by_senders": ["@foo:bar"]} async def events_have_relations(*args, **kwargs): return ["$with_relation"] From fcb7c2ea5771de247e292fc5c2262cf32308216f Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 5 Apr 2022 12:07:00 -0400 Subject: [PATCH 3/4] Remove the experimental config flag. --- synapse/config/experimental.py | 2 -- synapse/events/utils.py | 4 ---- synapse/rest/client/versions.py | 1 - synapse/server.py | 2 +- synapse/storage/databases/main/relations.py | 21 +-------------------- 5 files changed, 2 insertions(+), 28 deletions(-) diff --git a/synapse/config/experimental.py b/synapse/config/experimental.py index 0dd4c5958111..f93028d91b53 100644 --- a/synapse/config/experimental.py +++ b/synapse/config/experimental.py @@ -26,8 +26,6 @@ class ExperimentalConfig(Config): def read_config(self, config: JsonDict, **kwargs: Any) -> None: experimental = config.get("experimental_features") or {} - # MSC3440 (thread relation) - self.msc3440_enabled: bool = experimental.get("msc3440_enabled", False) # MSC3666: including bundled relations in /search. self.msc3666_enabled: bool = experimental.get("msc3666_enabled", False) diff --git a/synapse/events/utils.py b/synapse/events/utils.py index 3203e11b087a..43c3241fb0d1 100644 --- a/synapse/events/utils.py +++ b/synapse/events/utils.py @@ -39,7 +39,6 @@ if TYPE_CHECKING: from synapse.handlers.relations import BundledAggregations - from synapse.server import HomeServer # Split strings on "." but not "\." This uses a negative lookbehind assertion for '\' @@ -396,9 +395,6 @@ class EventClientSerializer: clients. """ - def __init__(self, hs: "HomeServer"): - self._msc3440_enabled = hs.config.experimental.msc3440_enabled - def serialize_event( self, event: Union[JsonDict, EventBase], diff --git a/synapse/rest/client/versions.py b/synapse/rest/client/versions.py index 9a65aa484360..7ee6b5505b20 100644 --- a/synapse/rest/client/versions.py +++ b/synapse/rest/client/versions.py @@ -100,7 +100,6 @@ def on_GET(self, request: Request) -> Tuple[int, JsonDict]: # Adds support for jump to date endpoints (/timestamp_to_event) as per MSC3030 "org.matrix.msc3030": self.config.experimental.msc3030_enabled, # Adds support for thread relations, per MSC3440. - "org.matrix.msc3440": self.config.experimental.msc3440_enabled, "org.matrix.msc3440.stable": True, # TODO: remove when "v1.3" is added above }, }, diff --git a/synapse/server.py b/synapse/server.py index 380369db923e..37c72bd83a9f 100644 --- a/synapse/server.py +++ b/synapse/server.py @@ -758,7 +758,7 @@ def get_oidc_handler(self) -> "OidcHandler": @cache_in_self def get_event_client_serializer(self) -> EventClientSerializer: - return EventClientSerializer(self) + return EventClientSerializer() @cache_in_self def get_password_policy_handler(self) -> PasswordPolicyHandler: diff --git a/synapse/storage/databases/main/relations.py b/synapse/storage/databases/main/relations.py index 43ca17232081..70c89309ed26 100644 --- a/synapse/storage/databases/main/relations.py +++ b/synapse/storage/databases/main/relations.py @@ -14,7 +14,6 @@ import logging from typing import ( - TYPE_CHECKING, Collection, Dict, FrozenSet, @@ -32,20 +31,12 @@ from synapse.api.constants import RelationTypes from synapse.events import EventBase from synapse.storage._base import SQLBaseStore -from synapse.storage.database import ( - DatabasePool, - LoggingDatabaseConnection, - LoggingTransaction, - make_in_list_sql_clause, -) +from synapse.storage.database import LoggingTransaction, make_in_list_sql_clause from synapse.storage.databases.main.stream import generate_pagination_where_clause from synapse.storage.engines import PostgresEngine from synapse.types import JsonDict, RoomStreamToken, StreamToken from synapse.util.caches.descriptors import cached, cachedList -if TYPE_CHECKING: - from synapse.server import HomeServer - logger = logging.getLogger(__name__) @@ -63,16 +54,6 @@ class _RelatedEvent: class RelationsWorkerStore(SQLBaseStore): - def __init__( - self, - database: DatabasePool, - db_conn: LoggingDatabaseConnection, - hs: "HomeServer", - ): - super().__init__(database, db_conn, hs) - - self._msc3440_enabled = hs.config.experimental.msc3440_enabled - @cached(uncached_args=("event",), tree=True) async def get_relations_for_event( self, From c2b9868020186cc2064d82e5095f0e7246b3bab2 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 5 Apr 2022 12:12:30 -0400 Subject: [PATCH 4/4] Newsfragment --- changelog.d/12382.removal | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/12382.removal diff --git a/changelog.d/12382.removal b/changelog.d/12382.removal new file mode 100644 index 000000000000..eb91186340f7 --- /dev/null +++ b/changelog.d/12382.removal @@ -0,0 +1 @@ +Remove unstable identifiers from [MSC3440](https://github.com/matrix-org/matrix-doc/pull/3440).