Add plain-text handling for rich-text topics as per MSC3765 (#18195)

This implements
matrix-org/matrix-spec-proposals#3765 which is
already merged and, therefore, can use stable identifiers.

For `/publicRooms` and `/hierarchy`, the topic is read from the
eponymous field of the `current_state_events` table. Rather than
introduce further columns in this table, I changed the insertion /
update logic to write the plain-text topic from the rich topic into the
existing field. This will not take effect for existing rooms unless
their topic is changed. However, existing rooms shouldn't have rich
topics to begin with.

Similarly, for server-side search, I changed the insertion logic of the
`event_search` table to prefer the value from the rich topic. Again,
existing events shouldn't have rich topics and, therefore, don't need to
be migrated in the table.

Spec doc: https://spec.matrix.org/v1.15/client-server-api/#mroomtopic

Part of supporting Matrix v1.15:
https://spec.matrix.org/v1.15/client-server-api/#mroomtopic

Signed-off-by: Johannes Marbach <[email protected]>
Co-authored-by: Eric Eastwood <[email protected]>

Author: Johennes

changelog.d/18195.feature

@@ -0,0 +1 @@
+Add plain-text handling for rich-text topics as per [MSC3765](https://github.com/matrix-org/matrix-spec-proposals/pull/3765).

synapse/api/constants.py

@@ -262,6 +262,11 @@ class EventContentFields:
 
     TOMBSTONE_SUCCESSOR_ROOM: Final = "replacement_room"
 
+    # Used in m.room.topic events.
+    TOPIC: Final = "topic"
+    M_TOPIC: Final = "m.topic"
+    M_TEXT: Final = "m.text"
+
 
 class EventUnsignedContentFields:
     """Fields found inside the 'unsigned' data on events"""
@@ -270,6 +275,13 @@ class EventUnsignedContentFields:
     MEMBERSHIP: Final = "membership"
 
 
+class MTextFields:
+    """Fields found inside m.text content blocks."""
+
+    BODY: Final = "body"
+    MIMETYPE: Final = "mimetype"
+
+
 class RoomTypes:
     """Understood values of the room_type field of m.room.create events."""
 

synapse/config/workers.py

@@ -27,8 +27,6 @@
 import attr
 
 from synapse._pydantic_compat import (
-    BaseModel,
-    Extra,
     StrictBool,
     StrictInt,
     StrictStr,
@@ -47,6 +45,7 @@
     parse_listener_def,
 )
 from synapse.types import JsonDict
+from synapse.util.pydantic_models import ParseModel
 
 _DEPRECATED_WORKER_DUTY_OPTION_USED = """
 The '%s' configuration option is deprecated and will be removed in a future
@@ -90,30 +89,7 @@ def _instance_to_list_converter(obj: Union[str, List[str]]) -> List[str]:
     return obj
 
 
-class ConfigModel(BaseModel):
-    """A custom version of Pydantic's BaseModel which
-
-     - ignores unknown fields and
-     - does not allow fields to be overwritten after construction,
-
-    but otherwise uses Pydantic's default behaviour.
-
-    For now, ignore unknown fields. In the future, we could change this so that unknown
-    config values cause a ValidationError, provided the error messages are meaningful to
-    server operators.
-
-    Subclassing in this way is recommended by
-    https://pydantic-docs.helpmanual.io/usage/model_config/#change-behaviour-globally
-    """
-
-    class Config:
-        # By default, ignore fields that we don't recognise.
-        extra = Extra.ignore
-        # By default, don't allow fields to be reassigned after parsing.
-        allow_mutation = False
-
-
-class InstanceTcpLocationConfig(ConfigModel):
+class InstanceTcpLocationConfig(ParseModel):
     """The host and port to talk to an instance via HTTP replication."""
 
     host: StrictStr
@@ -129,7 +105,7 @@ def netloc(self) -> str:
         return f"{self.host}:{self.port}"
 
 
-class InstanceUnixLocationConfig(ConfigModel):
+class InstanceUnixLocationConfig(ParseModel):
     """The socket file to talk to an instance via HTTP replication."""
 
     path: StrictStr

synapse/handlers/room.py

@@ -51,6 +51,7 @@
     HistoryVisibility,
     JoinRules,
     Membership,
+    MTextFields,
     RoomCreationPreset,
     RoomEncryptionAlgorithms,
     RoomTypes,
@@ -1303,7 +1304,13 @@ async def create_event(
             topic = room_config["topic"]
             topic_event, topic_context = await create_event(
                 EventTypes.Topic,
-                {"topic": topic},
+                {
+                    EventContentFields.TOPIC: topic,
+                    EventContentFields.M_TOPIC: {
+                        # The mimetype property defaults to `text/plain` if omitted.
+                        EventContentFields.M_TEXT: [{MTextFields.BODY: topic}]
+                    },
+                },
                 True,
             )
             events_to_send.append((topic_event, topic_context))

synapse/handlers/stats.py

@@ -36,6 +36,7 @@
 from synapse.metrics.background_process_metrics import run_as_background_process
 from synapse.storage.databases.main.state_deltas import StateDelta
 from synapse.types import JsonDict
+from synapse.util.events import get_plain_text_topic_from_event_content
 
 if TYPE_CHECKING:
     from synapse.server import HomeServer
@@ -299,7 +300,9 @@ async def _handle_deltas(
             elif delta.event_type == EventTypes.Name:
                 room_state["name"] = event_content.get("name")
             elif delta.event_type == EventTypes.Topic:
-                room_state["topic"] = event_content.get("topic")
+                room_state["topic"] = get_plain_text_topic_from_event_content(
+                    event_content
+                )
             elif delta.event_type == EventTypes.RoomAvatar:
                 room_state["avatar"] = event_content.get("url")
             elif delta.event_type == EventTypes.CanonicalAlias:

synapse/storage/databases/main/events.py

@@ -78,6 +78,7 @@
 from synapse.types.handlers import SLIDING_SYNC_DEFAULT_BUMP_EVENT_TYPES
 from synapse.types.state import StateFilter
 from synapse.util import json_encoder
+from synapse.util.events import get_plain_text_topic_from_event_content
 from synapse.util.iterutils import batch_iter, sorted_topologically
 from synapse.util.stringutils import non_null_str_or_none
 
@@ -3102,7 +3103,10 @@ def _handle_redact_relations(
     def _store_room_topic_txn(self, txn: LoggingTransaction, event: EventBase) -> None:
         if isinstance(event.content.get("topic"), str):
             self.store_event_search_txn(
-                txn, event, "content.topic", event.content["topic"]
+                txn,
+                event,
+                "content.topic",
+                get_plain_text_topic_from_event_content(event.content) or "",
             )
 
     def _store_room_name_txn(self, txn: LoggingTransaction, event: EventBase) -> None:

synapse/storage/databases/main/search.py

@@ -49,6 +49,7 @@
 from synapse.storage.databases.main.events_worker import EventRedactBehaviour
 from synapse.storage.engines import PostgresEngine, Sqlite3Engine
 from synapse.types import JsonDict
+from synapse.util.events import get_plain_text_topic_from_event_content
 
 if TYPE_CHECKING:
     from synapse.server import HomeServer
@@ -212,7 +213,9 @@ def reindex_search_txn(txn: LoggingTransaction) -> int:
                         value = content["body"]
                     elif etype == "m.room.topic":
                         key = "content.topic"
-                        value = content["topic"]
+                        value = (
+                            get_plain_text_topic_from_event_content(content) or "",
+                        )
                     elif etype == "m.room.name":
                         key = "content.name"
                         value = content["name"]

synapse/storage/databases/main/stats.py

@@ -48,6 +48,7 @@
 from synapse.storage.databases.main.state_deltas import StateDeltasStore
 from synapse.types import JsonDict
 from synapse.util.caches.descriptors import cached
+from synapse.util.events import get_plain_text_topic_from_event_content
 
 if TYPE_CHECKING:
     from synapse.server import HomeServer
@@ -611,7 +612,9 @@ def _fetch_current_state_stats(
             elif event.type == EventTypes.Name:
                 room_state["name"] = event.content.get("name")
             elif event.type == EventTypes.Topic:
-                room_state["topic"] = event.content.get("topic")
+                room_state["topic"] = get_plain_text_topic_from_event_content(
+                    event.content
+                )
             elif event.type == EventTypes.RoomAvatar:
                 room_state["avatar"] = event.content.get("url")
             elif event.type == EventTypes.CanonicalAlias:

synapse/types/rest/__init__.py

@@ -18,26 +18,8 @@
 # [This file includes modifications made by New Vector Limited]
 #
 #
-from synapse._pydantic_compat import BaseModel, Extra
+from synapse.util.pydantic_models import ParseModel
 
 
-class RequestBodyModel(BaseModel):
-    """A custom version of Pydantic's BaseModel which
-
-     - ignores unknown fields and
-     - does not allow fields to be overwritten after construction,
-
-    but otherwise uses Pydantic's default behaviour.
-
-    Ignoring unknown fields is a useful default. It means that clients can provide
-    unstable field not known to the server without the request being refused outright.
-
-    Subclassing in this way is recommended by
-    https://pydantic-docs.helpmanual.io/usage/model_config/#change-behaviour-globally
-    """
-
-    class Config:
-        # By default, ignore fields that we don't recognise.
-        extra = Extra.ignore
-        # By default, don't allow fields to be reassigned after parsing.
-        allow_mutation = False
+class RequestBodyModel(ParseModel):
+    pass

synapse/util/events.py

@@ -13,6 +13,11 @@
 #
 #
 
+from typing import Any, List, Optional
+
+from synapse._pydantic_compat import Field, StrictStr, ValidationError, validator
+from synapse.types import JsonDict
+from synapse.util.pydantic_models import ParseModel
 from synapse.util.stringutils import random_string
 
 
@@ -27,3 +32,100 @@ def generate_fake_event_id() -> str:
         A string intended to look like an event ID, but with no actual meaning.
     """
     return "$" + random_string(43)
+
+
+class MTextRepresentation(ParseModel):
+    """
+    See `TextualRepresentation` in the Matrix specification.
+    """
+
+    body: StrictStr
+    mimetype: Optional[StrictStr]
+
+
+class MTopic(ParseModel):
+    """
+    `m.room.topic` -> `content` -> `m.topic`
+
+    Textual representation of the room topic in different mimetypes. Added in Matrix v1.15.
+
+    See `TopicContentBlock` in the Matrix specification.
+    """
+
+    m_text: Optional[List[MTextRepresentation]] = Field(alias="m.text")
+    """
+    An ordered array of textual representations in different mimetypes.
+    """
+
+    # Because "Receivers SHOULD use the first representation in the array that they
+    # understand.", we ignore invalid representations in the `m.text` field and use
+    # what we can.
+    @validator("m_text", pre=True)
+    def ignore_invalid_representations(
+        cls, m_text: Any
+    ) -> Optional[List[MTextRepresentation]]:
+        if not isinstance(m_text, list):
+            raise ValueError("m.text must be a list")
+        representations = []
+        for element in m_text:
+            try:
+                representations.append(MTextRepresentation.parse_obj(element))
+            except ValidationError:
+                continue
+        return representations
+
+
+class TopicContent(ParseModel):
+    """
+    Represents the `content` field of an `m.room.topic` event
+    """
+
+    topic: StrictStr
+    """
+    The topic in plain text.
+    """
+
+    m_topic: Optional[MTopic] = Field(alias="m.topic")
+    """
+    Textual representation of the room topic in different mimetypes.
+    """
+
+    # We ignore invalid `m.topic` fields as we can always fall back to the plain-text
+    # `topic` field.
+    @validator("m_topic", pre=True)
+    def ignore_invalid_m_topic(cls, m_topic: Any) -> Optional[MTopic]:
+        try:
+            return MTopic.parse_obj(m_topic)
+        except ValidationError:
+            return None
+
+
+def get_plain_text_topic_from_event_content(content: JsonDict) -> Optional[str]:
+    """
+    Given the `content` of an `m.room.topic` event, returns the plain-text topic
+    representation. Prefers pulling plain-text from the newer `m.topic` field if
+    available with a fallback to `topic`.
+
+    Args:
+        content: The `content` field of an `m.room.topic` event.
+
+    Returns:
+        A string representing the plain text topic.
+    """
+
+    try:
+        topic_content = TopicContent.parse_obj(content)
+    except ValidationError:
+        return None
+
+    # Find the first `text/plain` topic ("Receivers SHOULD use the first
+    # representationin the array that they understand.")
+    if topic_content.m_topic and topic_content.m_topic.m_text:
+        for representation in topic_content.m_topic.m_text:
+            # The mimetype property defaults to `text/plain` if omitted.
+            if not representation.mimetype or representation.mimetype == "text/plain":
+                return representation.body
+
+    # Fallback to the plain-old `topic` field if there isn't any `text/plain` topic
+    # representation available.
+    return topic_content.topic