Re-emit room state events on rooms (#2607)

* Re-emit room state events on rooms This also fixes some potential memory leaks and abuse of removeAllListeners in sync.ts. * Remove some stray whitespace * Deduplicate some code to appease SonarCloud * Name helper function more explicitly
matrix-org · Aug 22, 2022 · b265d79 · b265d79
1 parent eb79f62
commit b265d79
Show file tree

Hide file tree

Showing 5 changed files with 140 additions and 145 deletions.
diff --git a/.eslintrc.js b/.eslintrc.js
@@ -57,6 +57,10 @@ module.exports = {
             // We're okay with assertion errors when we ask for them
             "@typescript-eslint/no-non-null-assertion": "off",
 
+            // The non-TypeScript rule produces false positives
+            "func-call-spacing": "off",
+            "@typescript-eslint/func-call-spacing": ["error"],
+
             "quotes": "off",
             // We use a `logger` intermediary module
             "no-console": "error",

diff --git a/src/ReEmitter.ts b/src/ReEmitter.ts
@@ -24,7 +24,16 @@ import { ListenerMap, TypedEventEmitter } from "./models/typed-event-emitter";
 export class ReEmitter {
     constructor(private readonly target: EventEmitter) {}
 
+    // Map from emitter to event name to re-emitter
+    private reEmitters = new Map<EventEmitter, Map<string, (...args: any[]) => void>>();
+
     public reEmit(source: EventEmitter, eventNames: string[]): void {
+        let reEmittersByEvent = this.reEmitters.get(source);
+        if (!reEmittersByEvent) {
+            reEmittersByEvent = new Map();
+            this.reEmitters.set(source, reEmittersByEvent);
+        }
+
         for (const eventName of eventNames) {
             // We include the source as the last argument for event handlers which may need it,
             // such as read receipt listeners on the client class which won't have the context
@@ -44,7 +53,20 @@ export class ReEmitter {
                 this.target.emit(eventName, ...args, source);
             };
             source.on(eventName, forSource);
+            reEmittersByEvent.set(eventName, forSource);
+        }
+    }
+
+    public stopReEmitting(source: EventEmitter, eventNames: string[]): void {
+        const reEmittersByEvent = this.reEmitters.get(source);
+        if (!reEmittersByEvent) return; // We were never re-emitting these events in the first place
+
+        for (const eventName of eventNames) {
+            source.off(eventName, reEmittersByEvent.get(eventName));
+            reEmittersByEvent.delete(eventName);
         }
+
+        if (reEmittersByEvent.size === 0) this.reEmitters.delete(source);
     }
 }
 
@@ -62,4 +84,11 @@ export class TypedReEmitter<
     ): void {
         super.reEmit(source, eventNames);
     }
+
+    public stopReEmitting<ReEmittedEvents extends string, T extends Events & ReEmittedEvents>(
+        source: TypedEventEmitter<ReEmittedEvents, any>,
+        eventNames: T[],
+    ): void {
+        super.stopReEmitting(source, eventNames);
+    }
 }
diff --git a/src/models/room.ts b/src/models/room.ts
@@ -1,5 +1,5 @@
 /*
-Copyright 2015 - 2021 The Matrix.org Foundation C.I.C.
+Copyright 2015 - 2022 The Matrix.org Foundation C.I.C.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -37,7 +37,8 @@ import {
 import { IRoomVersionsCapability, MatrixClient, PendingEventOrdering, RoomVersionStability } from "../client";
 import { GuestAccess, HistoryVisibility, JoinRule, ResizeMethod } from "../@types/partials";
 import { Filter, IFilterDefinition } from "../filter";
-import { RoomState } from "./room-state";
+import { RoomState, RoomStateEvent, RoomStateEventHandlerMap } from "./room-state";
+import { BeaconEvent, BeaconEventHandlerMap } from "./beacon";
 import {
     Thread,
     ThreadEvent,
@@ -172,16 +173,19 @@ export enum RoomEvent {
 }
 
 type EmittedEvents = RoomEvent
+    | RoomStateEvent.Events
+    | RoomStateEvent.Members
+    | RoomStateEvent.NewMember
+    | RoomStateEvent.Update
+    | RoomStateEvent.Marker
     | ThreadEvent.New
     | ThreadEvent.Update
     | ThreadEvent.NewReply
-    | RoomEvent.Timeline
-    | RoomEvent.TimelineReset
-    | RoomEvent.TimelineRefresh
-    | RoomEvent.HistoryImportedWithinTimeline
-    | RoomEvent.OldStateUpdated
-    | RoomEvent.CurrentStateUpdated
-    | MatrixEventEvent.BeforeRedaction;
+    | MatrixEventEvent.BeforeRedaction
+    | BeaconEvent.New
+    | BeaconEvent.Update
+    | BeaconEvent.Destroy
+    | BeaconEvent.LivenessChange;
 
 export type RoomEventHandlerMap = {
     [RoomEvent.MyMembership]: (room: Room, membership: string, prevMembership?: string) => void;
@@ -205,7 +209,21 @@ export type RoomEventHandlerMap = {
     ) => void;
     [RoomEvent.TimelineRefresh]: (room: Room, eventTimelineSet: EventTimelineSet) => void;
     [ThreadEvent.New]: (thread: Thread, toStartOfTimeline: boolean) => void;
-} & ThreadHandlerMap & MatrixEventHandlerMap;
+} & ThreadHandlerMap
+    & MatrixEventHandlerMap
+    & Pick<
+        RoomStateEventHandlerMap,
+        RoomStateEvent.Events
+            | RoomStateEvent.Members
+            | RoomStateEvent.NewMember
+            | RoomStateEvent.Update
+            | RoomStateEvent.Marker
+            | BeaconEvent.New
+    >
+    & Pick<
+        BeaconEventHandlerMap,
+        BeaconEvent.Update | BeaconEvent.Destroy | BeaconEvent.LivenessChange
+    >;
 
 export class Room extends TypedEventEmitter<EmittedEvents, RoomEventHandlerMap> {
     public readonly reEmitter: TypedReEmitter<EmittedEvents, RoomEventHandlerMap>;
@@ -1068,6 +1086,32 @@ export class Room extends TypedEventEmitter<EmittedEvents, RoomEventHandlerMap>
 
         if (previousCurrentState !== this.currentState) {
             this.emit(RoomEvent.CurrentStateUpdated, this, previousCurrentState, this.currentState);
+
+            // Re-emit various events on the current room state
+            // TODO: If currentState really only exists for backwards
+            // compatibility, shouldn't we be doing this some other way?
+            this.reEmitter.stopReEmitting(previousCurrentState, [
+                RoomStateEvent.Events,
+                RoomStateEvent.Members,
+                RoomStateEvent.NewMember,
+                RoomStateEvent.Update,
+                RoomStateEvent.Marker,
+                BeaconEvent.New,
+                BeaconEvent.Update,
+                BeaconEvent.Destroy,
+                BeaconEvent.LivenessChange,
+            ]);
+            this.reEmitter.reEmit(this.currentState, [
+                RoomStateEvent.Events,
+                RoomStateEvent.Members,
+                RoomStateEvent.NewMember,
+                RoomStateEvent.Update,
+                RoomStateEvent.Marker,
+                BeaconEvent.New,
+                BeaconEvent.Update,
+                BeaconEvent.Destroy,
+                BeaconEvent.LivenessChange,
+            ]);
         }
     }
 

diff --git a/src/sliding-sync-sdk.ts b/src/sliding-sync-sdk.ts
@@ -19,13 +19,11 @@ import { logger } from './logger';
 import * as utils from "./utils";
 import { EventTimeline } from "./models/event-timeline";
 import { ClientEvent, IStoredClientOpts, MatrixClient, PendingEventOrdering } from "./client";
-import { ISyncStateData, SyncState } from "./sync";
+import { ISyncStateData, SyncState, _createAndReEmitRoom } from "./sync";
 import { MatrixEvent } from "./models/event";
 import { Crypto } from "./crypto";
 import { IMinimalEvent, IRoomEvent, IStateEvent, IStrippedState } from "./sync-accumulator";
 import { MatrixError } from "./http-api";
-import { RoomStateEvent } from "./models/room-state";
-import { RoomMemberEvent } from "./models/room-member";
 import {
     Extension,
     ExtensionState,
@@ -290,7 +288,7 @@ export class SlidingSyncSdk {
                 logger.debug("initial flag not set but no stored room exists for room ", roomId, roomData);
                 return;
             }
-            room = createRoom(this.client, roomId, this.opts);
+            room = _createAndReEmitRoom(this.client, roomId, this.opts);
         }
         this.processRoomData(this.client, room, roomData);
     }
@@ -536,7 +534,6 @@ export class SlidingSyncSdk {
             }
 
             if (limited) {
-                deregisterStateListeners(room);
                 room.resetLiveTimeline(
                     roomData.prev_batch,
                     null, // TODO this.opts.canResetEntireTimeline(room.roomId) ? null : syncEventData.oldSyncToken,
@@ -546,7 +543,6 @@ export class SlidingSyncSdk {
                 // reason to stop incrementally tracking notifications and
                 // reset the timeline.
                 this.client.resetNotifTimelineSet();
-                registerStateListeners(this.client, room);
             }
         } */
 
@@ -816,58 +812,6 @@ function ensureNameEvent(client: MatrixClient, roomId: string, roomData: MSC3575
 // Helper functions which set up JS SDK structs are below and are identical to the sync v2 counterparts,
 // just outside the class.
 
-function createRoom(client: MatrixClient, roomId: string, opts: Partial<IStoredClientOpts>): Room { // XXX cargoculted from sync.ts
-    const { timelineSupport } = client;
-    const room = new Room(roomId, client, client.getUserId(), {
-        lazyLoadMembers: opts.lazyLoadMembers,
-        pendingEventOrdering: opts.pendingEventOrdering,
-        timelineSupport,
-    });
-    client.reEmitter.reEmit(room, [
-        RoomEvent.Name,
-        RoomEvent.Redaction,
-        RoomEvent.RedactionCancelled,
-        RoomEvent.Receipt,
-        RoomEvent.Tags,
-        RoomEvent.LocalEchoUpdated,
-        RoomEvent.AccountData,
-        RoomEvent.MyMembership,
-        RoomEvent.Timeline,
-        RoomEvent.TimelineReset,
-    ]);
-    registerStateListeners(client, room);
-    return room;
-}
-
-function registerStateListeners(client: MatrixClient, room: Room): void { // XXX cargoculted from sync.ts
-    // we need to also re-emit room state and room member events, so hook it up
-    // to the client now. We need to add a listener for RoomState.members in
-    // order to hook them correctly.
-    client.reEmitter.reEmit(room.currentState, [
-        RoomStateEvent.Events,
-        RoomStateEvent.Members,
-        RoomStateEvent.NewMember,
-        RoomStateEvent.Update,
-    ]);
-    room.currentState.on(RoomStateEvent.NewMember, function(event, state, member) {
-        member.user = client.getUser(member.userId);
-        client.reEmitter.reEmit(member, [
-            RoomMemberEvent.Name,
-            RoomMemberEvent.Typing,
-            RoomMemberEvent.PowerLevel,
-            RoomMemberEvent.Membership,
-        ]);
-    });
-}
-
-/*
-function deregisterStateListeners(room: Room): void { // XXX cargoculted from sync.ts
-    // could do with a better way of achieving this.
-    room.currentState.removeAllListeners(RoomStateEvent.Events);
-    room.currentState.removeAllListeners(RoomStateEvent.Members);
-    room.currentState.removeAllListeners(RoomStateEvent.NewMember);
-} */
-
 function mapEvents(client: MatrixClient, roomId: string, events: object[], decrypt = true): MatrixEvent[] {
     const mapper = client.getEventMapper({ decrypt });
     return (events as Array<IStrippedState | IRoomEvent | IStateEvent | IMinimalEvent>).map(function(e) {