From c93128ed3925393078ece0738bf8c6e8f74bb921 Mon Sep 17 00:00:00 2001
From: Will Hunt <will@half-shot.uk>
Date: Mon, 3 Feb 2025 08:13:44 +0000
Subject: [PATCH] Handle empty m.room.topic (#4673)

* Define topic as optional.

* Change isProvided so that types work better.

* allow makeTopicContent and parseTopicContent to handle optional values for plain text

* linting

* Remove usage of optional

* Topic key may only contain legacy key.

* Add tests for other branches.
---
 spec/unit/content-helpers.spec.ts     | 32 +++++++++++++++++++++++++++
 src/@types/state_events.ts            |  2 +-
 src/@types/topic.ts                   |  2 +-
 src/client.ts                         |  4 +++-
 src/content-helpers.ts                | 16 +++++++++-----
 src/extensible_events_v1/utilities.ts |  2 +-
 6 files changed, 48 insertions(+), 10 deletions(-)

diff --git a/spec/unit/content-helpers.spec.ts b/spec/unit/content-helpers.spec.ts
index afb465f53..79c2f7726 100644
--- a/spec/unit/content-helpers.spec.ts
+++ b/spec/unit/content-helpers.spec.ts
@@ -207,6 +207,17 @@ describe("Topic content helpers", () => {
                 ],
             });
         });
+
+        it("creates an empty event when the topic is falsey", () => {
+            expect(makeTopicContent(undefined)).toEqual({
+                topic: undefined,
+                [M_TOPIC.name]: [],
+            });
+            expect(makeTopicContent(null)).toEqual({
+                topic: null,
+                [M_TOPIC.name]: [],
+            });
+        });
     });
 
     describe("parseTopicContent()", () => {
@@ -257,5 +268,26 @@ describe("Topic content helpers", () => {
                 html: "<b>pizza</b>",
             });
         });
+
+        it("parses legacy event content", () => {
+            expect(
+                parseTopicContent({
+                    topic: "pizza",
+                }),
+            ).toEqual({
+                text: "pizza",
+            });
+        });
+
+        it("uses legacy event content when new topic key is invalid", () => {
+            expect(
+                parseTopicContent({
+                    "topic": "pizza",
+                    "m.topic": {} as any,
+                }),
+            ).toEqual({
+                text: "pizza",
+            });
+        });
     });
 });
diff --git a/src/@types/state_events.ts b/src/@types/state_events.ts
index d570c5a12..a580c41c4 100644
--- a/src/@types/state_events.ts
+++ b/src/@types/state_events.ts
@@ -90,7 +90,7 @@ export interface RoomNameEventContent {
 }
 
 export interface RoomTopicEventContent {
-    topic: string;
+    topic: string | undefined | null;
 }
 
 export interface RoomAvatarEventContent {
diff --git a/src/@types/topic.ts b/src/@types/topic.ts
index 3d4a63332..6254191e8 100644
--- a/src/@types/topic.ts
+++ b/src/@types/topic.ts
@@ -60,4 +60,4 @@ export type MTopicEvent = EitherAnd<{ [M_TOPIC.name]: MTopicContent }, { [M_TOPI
 /**
  * The event content for an m.room.topic event
  */
-export type MRoomTopicEventContent = { topic: string } & MTopicEvent;
+export type MRoomTopicEventContent = { topic: string | null | undefined } & (MTopicEvent | {});
diff --git a/src/client.ts b/src/client.ts
index 5e1c8deee..c05e6a1a1 100644
--- a/src/client.ts
+++ b/src/client.ts
@@ -4490,11 +4490,13 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
     }
 
     /**
+     * @param roomId - The room to update the topic in.
+     * @param topic - The plaintext topic. May be empty to remove the topic.
      * @param htmlTopic - Optional.
      * @returns Promise which resolves: TODO
      * @returns Rejects: with an error response.
      */
-    public setRoomTopic(roomId: string, topic: string, htmlTopic?: string): Promise<ISendEventResponse> {
+    public setRoomTopic(roomId: string, topic?: string, htmlTopic?: string): Promise<ISendEventResponse> {
         const content = ContentHelpers.makeTopicContent(topic, htmlTopic);
         return this.sendStateEvent(roomId, EventType.RoomTopic, content);
     }
diff --git a/src/content-helpers.ts b/src/content-helpers.ts
index ce8b373d9..d2fe37add 100644
--- a/src/content-helpers.ts
+++ b/src/content-helpers.ts
@@ -185,27 +185,31 @@ export const parseLocationEvent = (wireEventContent: LocationEventWireContent):
 /**
  * Topic event helpers
  */
-export type MakeTopicContent = (topic: string, htmlTopic?: string) => MRoomTopicEventContent;
+export type MakeTopicContent = (topic: string | null | undefined, htmlTopic?: string) => MRoomTopicEventContent;
 
 export const makeTopicContent: MakeTopicContent = (topic, htmlTopic) => {
-    const renderings = [{ body: topic, mimetype: "text/plain" }];
+    const renderings = [];
+    if (isProvided(topic)) {
+        renderings.push({ body: topic, mimetype: "text/plain" });
+    }
     if (isProvided(htmlTopic)) {
-        renderings.push({ body: htmlTopic!, mimetype: "text/html" });
+        renderings.push({ body: htmlTopic, mimetype: "text/html" });
     }
     return { topic, [M_TOPIC.name]: renderings };
 };
 
 export type TopicState = {
-    text: string;
+    text?: string;
     html?: string;
 };
 
 export const parseTopicContent = (content: MRoomTopicEventContent): TopicState => {
     const mtopic = M_TOPIC.findIn<MTopicContent>(content);
     if (!Array.isArray(mtopic)) {
-        return { text: content.topic };
+        return { text: content.topic ?? undefined };
     }
-    const text = mtopic?.find((r) => !isProvided(r.mimetype) || r.mimetype === "text/plain")?.body ?? content.topic;
+    const text =
+        mtopic?.find((r) => !isProvided(r.mimetype) || r.mimetype === "text/plain")?.body ?? content.topic ?? undefined;
     const html = mtopic?.find((r) => r.mimetype === "text/html")?.body;
     return { text, html };
 };
diff --git a/src/extensible_events_v1/utilities.ts b/src/extensible_events_v1/utilities.ts
index 0660442ec..49911dc68 100644
--- a/src/extensible_events_v1/utilities.ts
+++ b/src/extensible_events_v1/utilities.ts
@@ -21,7 +21,7 @@ import { Optional } from "matrix-events-sdk";
  * @param s - The optional to test.
  * @returns True if the value is defined.
  */
-export function isProvided<T>(s: Optional<T>): boolean {
+export function isProvided<T>(s: Optional<T>): s is T {
     return s !== null && s !== undefined;
 }