From 86cf6a3a17376dd83a9222f77521d1619b528d66 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 12 Apr 2022 08:42:03 -0400 Subject: [PATCH] Remove references to unstable identifiers from MSC3440. (#12382) Removes references to unstable thread relation, unstable identifiers for filtering parameters, and the experimental config flag. --- changelog.d/12382.removal | 1 + synapse/api/constants.py | 2 - synapse/api/filtering.py | 12 ---- synapse/config/experimental.py | 3 - synapse/events/utils.py | 6 -- synapse/handlers/message.py | 5 +- synapse/rest/client/versions.py | 1 - synapse/server.py | 2 +- synapse/storage/databases/main/events.py | 5 +- synapse/storage/databases/main/relations.py | 78 +++++---------------- tests/api/test_filtering.py | 4 +- 11 files changed, 21 insertions(+), 98 deletions(-) create mode 100644 changelog.d/12382.removal diff --git a/changelog.d/12382.removal b/changelog.d/12382.removal new file mode 100644 index 0000000000..eb91186340 --- /dev/null +++ b/changelog.d/12382.removal @@ -0,0 +1 @@ +Remove unstable identifiers from [MSC3440](https://github.com/matrix-org/matrix-doc/pull/3440). diff --git a/synapse/api/constants.py b/synapse/api/constants.py index 92907415e6..0172eb60b8 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/api/filtering.py b/synapse/api/filtering.py index 27e97d6f37..4a808e33fe 100644 --- a/synapse/api/filtering.py +++ b/synapse/api/filtering.py @@ -89,9 +89,7 @@ ROOM_EVENT_FILTER_SCHEMA = { "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 @@ class Filter: 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/synapse/config/experimental.py b/synapse/config/experimental.py index 447476fbfa..0772dce411 100644 --- a/synapse/config/experimental.py +++ b/synapse/config/experimental.py @@ -26,9 +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) - # MSC3026 (busy presence state) self.msc3026_enabled: bool = experimental.get("msc3026_enabled", False) diff --git a/synapse/events/utils.py b/synapse/events/utils.py index 918e87ed9c..43c3241fb0 100644 --- a/synapse/events/utils.py +++ b/synapse/events/utils.py @@ -39,7 +39,6 @@ from . import EventBase 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], @@ -525,8 +521,6 @@ class EventClientSerializer: "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 7db6905c61..47a63005a9 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -1102,10 +1102,7 @@ class EventCreationHandler: 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/rest/client/versions.py b/synapse/rest/client/versions.py index 9a65aa4843..7ee6b5505b 100644 --- a/synapse/rest/client/versions.py +++ b/synapse/rest/client/versions.py @@ -100,7 +100,6 @@ class VersionsRestServlet(RestServlet): # 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 380369db92..37c72bd83a 100644 --- a/synapse/server.py +++ b/synapse/server.py @@ -758,7 +758,7 @@ class HomeServer(metaclass=abc.ABCMeta): @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/events.py b/synapse/storage/databases/main/events.py index 3fcd5f5b99..e3be537cee 100644 --- a/synapse/storage/databases/main/events.py +++ b/synapse/storage/databases/main/events.py @@ -1819,10 +1819,7 @@ class PersistEventsStore: 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 407158ceee..a5c31f6787 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 @@ import attr 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, @@ -497,7 +478,7 @@ class RelationsWorkerStore(SQLBaseStore): 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 +493,16 @@ class RelationsWorkerStore(SQLBaseStore): 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 +522,7 @@ class RelationsWorkerStore(SQLBaseStore): AND parent.room_id = child.room_id WHERE %s - AND %s + AND relation_type = ? GROUP BY parent.event_id """ @@ -556,15 +531,9 @@ class RelationsWorkerStore(SQLBaseStore): 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 +591,7 @@ class RelationsWorkerStore(SQLBaseStore): 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 +607,9 @@ class RelationsWorkerStore(SQLBaseStore): 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 +639,7 @@ class RelationsWorkerStore(SQLBaseStore): 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 +650,20 @@ class RelationsWorkerStore(SQLBaseStore): 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} diff --git a/tests/api/test_filtering.py b/tests/api/test_filtering.py index 8c3354ce3c..985d6e397d 100644 --- a/tests/api/test_filtering.py +++ b/tests/api/test_filtering.py @@ -481,9 +481,7 @@ class FilteringTestCase(unittest.HomeserverTestCase): # 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"]