Factor SyncApi options out of IStoredClientOptions (#3009)

There are a couple of callback interfaces which are currently stuffed into
`IStoredClientOpts` to make it easier to pass them into the `SyncApi`
constructor.

Before we add more fields to this, let's separate it out to a separate object.

spec/integ/matrix-client-methods.spec.ts

@@ -35,9 +35,7 @@ describe("MatrixClient", function () {
     let store: MemoryStore | undefined;
 
     const defaultClientOpts: IStoredClientOpts = {
-        canResetEntireTimeline: (roomId) => false,
         experimentalThreadSupport: false,
-        crypto: {} as unknown as IStoredClientOpts["crypto"],
     };
     const setupTests = (): [MatrixClient, HttpBackend, MemoryStore] => {
         const store = new MemoryStore();

spec/integ/sliding-sync-sdk.spec.ts

@@ -38,7 +38,7 @@ import {
     IRoomTimelineData,
 } from "../../src";
 import { SlidingSyncSdk } from "../../src/sliding-sync-sdk";
-import { SyncState } from "../../src/sync";
+import { SyncApiOptions, SyncState } from "../../src/sync";
 import { IStoredClientOpts } from "../../src/client";
 import { logger } from "../../src/logger";
 import { emitPromise } from "../test-utils/test-utils";
@@ -111,6 +111,7 @@ describe("SlidingSyncSdk", () => {
     // assign client/httpBackend globals
     const setupClient = async (testOpts?: Partial<IStoredClientOpts & { withCrypto: boolean }>) => {
         testOpts = testOpts || {};
+        const syncOpts: SyncApiOptions = {};
         const testClient = new TestClient(selfUserId, "DEVICE", selfAccessToken);
         httpBackend = testClient.httpBackend;
         client = testClient.client;
@@ -118,10 +119,10 @@ describe("SlidingSyncSdk", () => {
         if (testOpts.withCrypto) {
             httpBackend!.when("GET", "/room_keys/version").respond(404, {});
             await client!.initCrypto();
-            testOpts.crypto = client!.crypto;
+            syncOpts.crypto = client!.crypto;
         }
         httpBackend!.when("GET", "/_matrix/client/r0/pushrules").respond(200, {});
-        sdk = new SlidingSyncSdk(mockSlidingSync, client, testOpts);
+        sdk = new SlidingSyncSdk(mockSlidingSync, client, testOpts, syncOpts);
     };
 
     // tear down client/httpBackend globals

src/client.ts

@@ -21,7 +21,7 @@ limitations under the License.
 import { EmoteEvent, IPartialEvent, MessageEvent, NoticeEvent, Optional } from "matrix-events-sdk";
 
 import type { IMegolmSessionData } from "./@types/crypto";
-import { ISyncStateData, SyncApi, SyncState } from "./sync";
+import { ISyncStateData, SyncApi, SyncApiOptions, SyncState } from "./sync";
 import {
     EventStatus,
     IContent,
@@ -456,17 +456,7 @@ export interface IStartClientOpts {
     slidingSync?: SlidingSync;
 }
 
-export interface IStoredClientOpts extends IStartClientOpts {
+export interface IStoredClientOpts extends IStartClientOpts {}
-    // Crypto manager
-    crypto?: Crypto;
-    /**
-     * A function which is called
-     * with a room ID and returns a boolean. It should return 'true' if the SDK can
-     * SAFELY remove events from this room. It may not be safe to remove events if
-     * there are other references to the timelines for this room.
-     */
-    canResetEntireTimeline: ResetTimelineCallback;
-}
 
 export enum RoomVersionStability {
     Stable = "stable",
@@ -1433,20 +1423,18 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
             logger.error("Can't fetch server versions, continuing to initialise sync, this will be retried later", e);
         }
 
-        // shallow-copy the opts dict before modifying and storing it
+        this.clientOpts = opts ?? {};
-        this.clientOpts = Object.assign({}, opts) as IStoredClientOpts;
-        this.clientOpts.crypto = this.crypto;
-        this.clientOpts.canResetEntireTimeline = (roomId): boolean => {
-            if (!this.canResetTimelineCallback) {
-                return false;
-            }
-            return this.canResetTimelineCallback(roomId);
-        };
         if (this.clientOpts.slidingSync) {
-            this.syncApi = new SlidingSyncSdk(this.clientOpts.slidingSync, this, this.clientOpts);
+            this.syncApi = new SlidingSyncSdk(
+                this.clientOpts.slidingSync,
+                this,
+                this.clientOpts,
+                this.buildSyncApiOptions(),
+            );
         } else {
-            this.syncApi = new SyncApi(this, this.clientOpts);
+            this.syncApi = new SyncApi(this, this.clientOpts, this.buildSyncApiOptions());
         }
+
         this.syncApi.sync();
 
         if (this.clientOpts.clientWellKnownPollPeriod !== undefined) {
@@ -1459,6 +1447,21 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
         this.toDeviceMessageQueue.start();
     }
 
+    /**
+     * Construct a SyncApiOptions for this client, suitable for passing into the SyncApi constructor
+     */
+    protected buildSyncApiOptions(): SyncApiOptions {
+        return {
+            crypto: this.crypto,
+            canResetEntireTimeline: (roomId: string): boolean => {
+                if (!this.canResetTimelineCallback) {
+                    return false;
+                }
+                return this.canResetTimelineCallback(roomId);
+            },
+        };
+    }
+
     /**
      * High level helper method to stop the client from polling and allow a
      * clean shutdown.
@@ -3954,7 +3957,7 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
             const res = await this.http.authedRequest<{ room_id: string }>(Method.Post, path, queryString, data);
 
             const roomId = res.room_id;
-            const syncApi = new SyncApi(this, this.clientOpts);
+            const syncApi = new SyncApi(this, this.clientOpts, this.buildSyncApiOptions());
             const room = syncApi.createRoom(roomId);
             if (opts.syncRoom) {
                 // v2 will do this for us
@@ -6154,7 +6157,7 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
      */
     public peekInRoom(roomId: string): Promise<Room> {
         this.peekSync?.stopPeeking();
-        this.peekSync = new SyncApi(this, this.clientOpts);
+        this.peekSync = new SyncApi(this, this.clientOpts, this.buildSyncApiOptions());
         return this.peekSync.peek(roomId);
     }
 
@@ -6661,7 +6664,7 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
         if (this.syncLeftRoomsPromise) {
             return this.syncLeftRoomsPromise; // return the ongoing request
         }
-        const syncApi = new SyncApi(this, this.clientOpts);
+        const syncApi = new SyncApi(this, this.clientOpts, this.buildSyncApiOptions());
         this.syncLeftRoomsPromise = syncApi.syncLeftRooms();
 
         // cleanup locks

src/embedded.ts

@@ -167,9 +167,9 @@ export class RoomWidgetClient extends MatrixClient {
         // still has some valuable helper methods that we make use of, so we
         // instantiate it anyways
         if (opts.slidingSync) {
-            this.syncApi = new SlidingSyncSdk(opts.slidingSync, this, opts);
+            this.syncApi = new SlidingSyncSdk(opts.slidingSync, this, opts, this.buildSyncApiOptions());
         } else {
-            this.syncApi = new SyncApi(this, opts);
+            this.syncApi = new SyncApi(this, opts, this.buildSyncApiOptions());
         }
 
         this.room = this.syncApi.createRoom(this.roomId);

src/sliding-sync-sdk.ts

@@ -18,8 +18,15 @@ import { NotificationCountType, Room, RoomEvent } from "./models/room";
 import { logger } from "./logger";
 import * as utils from "./utils";
 import { EventTimeline } from "./models/event-timeline";
-import { ClientEvent, IStoredClientOpts, MatrixClient, PendingEventOrdering } from "./client";
+import { ClientEvent, IStoredClientOpts, MatrixClient } from "./client";
-import { ISyncStateData, SyncState, _createAndReEmitRoom } from "./sync";
+import {
+    ISyncStateData,
+    SyncState,
+    _createAndReEmitRoom,
+    SyncApiOptions,
+    defaultClientOpts,
+    defaultSyncApiOpts,
+} from "./sync";
 import { MatrixEvent } from "./models/event";
 import { Crypto } from "./crypto";
 import { IMinimalEvent, IRoomEvent, IStateEvent, IStrippedState, ISyncResponse } from "./sync-accumulator";
@@ -342,6 +349,8 @@ class ExtensionReceipts implements Extension<ExtensionReceiptsRequest, Extension
  * sliding sync API, see sliding-sync.ts or the class SlidingSync.
  */
 export class SlidingSyncSdk {
+    private readonly opts: IStoredClientOpts;
+    private readonly syncOpts: SyncApiOptions;
     private syncState: SyncState | null = null;
     private syncStateData?: ISyncStateData;
     private lastPos: string | null = null;
@@ -351,19 +360,11 @@ export class SlidingSyncSdk {
     public constructor(
         private readonly slidingSync: SlidingSync,
         private readonly client: MatrixClient,
-        private readonly opts: Partial<IStoredClientOpts> = {},
+        opts?: IStoredClientOpts,
+        syncOpts?: SyncApiOptions,
     ) {
-        this.opts.initialSyncLimit = this.opts.initialSyncLimit ?? 8;
+        this.opts = defaultClientOpts(opts);
-        this.opts.resolveInvitesToProfiles = this.opts.resolveInvitesToProfiles || false;
+        this.syncOpts = defaultSyncApiOpts(syncOpts);
-        this.opts.pollTimeout = this.opts.pollTimeout || 30 * 1000;
-        this.opts.pendingEventOrdering = this.opts.pendingEventOrdering || PendingEventOrdering.Chronological;
-        this.opts.experimentalThreadSupport = this.opts.experimentalThreadSupport === true;
-
-        if (!opts.canResetEntireTimeline) {
-            opts.canResetEntireTimeline = (_roomId: string): boolean => {
-                return false;
-            };
-        }
 
         if (client.getNotifTimelineSet()) {
             client.reEmitter.reEmit(client.getNotifTimelineSet()!, [RoomEvent.Timeline, RoomEvent.TimelineReset]);
@@ -377,8 +378,8 @@ export class SlidingSyncSdk {
             new ExtensionTyping(this.client),
             new ExtensionReceipts(this.client),
         ];
-        if (this.opts.crypto) {
+        if (this.syncOpts.crypto) {
-            extensions.push(new ExtensionE2EE(this.opts.crypto));
+            extensions.push(new ExtensionE2EE(this.syncOpts.crypto));
         }
         extensions.forEach((ext) => {
             this.slidingSync.registerExtension(ext);
@@ -698,7 +699,7 @@ export class SlidingSyncSdk {
             if (limited) {
                 room.resetLiveTimeline(
                     roomData.prev_batch,
-                    null, // TODO this.opts.canResetEntireTimeline(room.roomId) ? null : syncEventData.oldSyncToken,
+                    null, // TODO this.syncOpts.canResetEntireTimeline(room.roomId) ? null : syncEventData.oldSyncToken,
                 );
 
                 // We have to assume any gap in any timeline is
@@ -730,8 +731,8 @@ export class SlidingSyncSdk {
 
         const processRoomEvent = async (e: MatrixEvent): Promise<void> => {
             client.emit(ClientEvent.Event, e);
-            if (e.isState() && e.getType() == EventType.RoomEncryption && this.opts.crypto) {
+            if (e.isState() && e.getType() == EventType.RoomEncryption && this.syncOpts.crypto) {
-                await this.opts.crypto.onCryptoEvent(room, e);
+                await this.syncOpts.crypto.onCryptoEvent(room, e);
             }
         };
 

src/sync.ts

@@ -34,7 +34,7 @@ import { EventTimeline } from "./models/event-timeline";
 import { PushProcessor } from "./pushprocessor";
 import { logger } from "./logger";
 import { InvalidStoreError, InvalidStoreState } from "./errors";
-import { ClientEvent, IStoredClientOpts, MatrixClient, PendingEventOrdering } from "./client";
+import { ClientEvent, IStoredClientOpts, MatrixClient, PendingEventOrdering, ResetTimelineCallback } from "./client";
 import {
     IEphemeral,
     IInvitedRoom,
@@ -59,6 +59,7 @@ import { BeaconEvent } from "./models/beacon";
 import { IEventsResponse } from "./@types/requests";
 import { UNREAD_THREAD_NOTIFICATIONS } from "./@types/sync";
 import { Feature, ServerSupport } from "./feature";
+import { Crypto } from "./crypto";
 
 const DEBUG = true;
 
@@ -110,6 +111,22 @@ function debuglog(...params: any[]): void {
     logger.log(...params);
 }
 
+/**
+ * Options passed into the constructor of SyncApi by MatrixClient
+ */
+export interface SyncApiOptions {
+    // Crypto manager
+    crypto?: Crypto;
+
+    /**
+     * A function which is called
+     * with a room ID and returns a boolean. It should return 'true' if the SDK can
+     * SAFELY remove events from this room. It may not be safe to remove events if
+     * there are other references to the timelines for this room.
+     */
+    canResetEntireTimeline?: ResetTimelineCallback;
+}
+
 interface ISyncOptions {
     filter?: string;
     hasSyncedBefore?: boolean;
@@ -163,7 +180,29 @@ type WrappedRoom<T> = T & {
     isBrandNewRoom: boolean;
 };
 
+/** add default settings to an IStoredClientOpts */
+export function defaultClientOpts(opts?: IStoredClientOpts): IStoredClientOpts {
+    return {
+        initialSyncLimit: 8,
+        resolveInvitesToProfiles: false,
+        pollTimeout: 30 * 1000,
+        pendingEventOrdering: PendingEventOrdering.Chronological,
+        experimentalThreadSupport: false,
+        ...opts,
+    };
+}
+
+export function defaultSyncApiOpts(syncOpts?: SyncApiOptions): SyncApiOptions {
+    return {
+        canResetEntireTimeline: (_roomId): boolean => false,
+        ...syncOpts,
+    };
+}
+
 export class SyncApi {
+    private readonly opts: IStoredClientOpts;
+    private readonly syncOpts: SyncApiOptions;
+
     private _peekRoom: Optional<Room> = null;
     private currentSyncRequest?: Promise<ISyncResponse>;
     private abortController?: AbortController;
@@ -180,21 +219,13 @@ export class SyncApi {
     /**
      * Construct an entity which is able to sync with a homeserver.
      * @param client - The matrix client instance to use.
-     * @param opts - Config options
+     * @param opts - client config options
+     * @param syncOpts - sync-specific options passed by the client
      * @internal
      */
-    public constructor(private readonly client: MatrixClient, private readonly opts: Partial<IStoredClientOpts> = {}) {
+    public constructor(private readonly client: MatrixClient, opts?: IStoredClientOpts, syncOpts?: SyncApiOptions) {
-        this.opts.initialSyncLimit = this.opts.initialSyncLimit ?? 8;
+        this.opts = defaultClientOpts(opts);
-        this.opts.resolveInvitesToProfiles = this.opts.resolveInvitesToProfiles || false;
+        this.syncOpts = defaultSyncApiOpts(syncOpts);
-        this.opts.pollTimeout = this.opts.pollTimeout || 30 * 1000;
-        this.opts.pendingEventOrdering = this.opts.pendingEventOrdering || PendingEventOrdering.Chronological;
-        this.opts.experimentalThreadSupport = this.opts.experimentalThreadSupport === true;
-
-        if (!opts.canResetEntireTimeline) {
-            opts.canResetEntireTimeline = (roomId: string): boolean => {
-                return false;
-            };
-        }
 
         if (client.getNotifTimelineSet()) {
             client.reEmitter.reEmit(client.getNotifTimelineSet()!, [RoomEvent.Timeline, RoomEvent.TimelineReset]);
@@ -632,7 +663,7 @@ export class SyncApi {
             return;
         }
         if (this.opts.lazyLoadMembers) {
-            this.opts.crypto?.enableLazyLoading();
+            this.syncOpts.crypto?.enableLazyLoading();
         }
         try {
             debuglog("Storing client options...");
@@ -866,10 +897,10 @@ export class SyncApi {
                 catchingUp: this.catchingUp,
             };
 
-            if (this.opts.crypto) {
+            if (this.syncOpts.crypto) {
                 // tell the crypto module we're about to process a sync
                 // response
-                await this.opts.crypto.onSyncWillProcess(syncEventData);
+                await this.syncOpts.crypto.onSyncWillProcess(syncEventData);
             }
 
             try {
@@ -894,8 +925,8 @@ export class SyncApi {
 
             // tell the crypto module to do its processing. It may block (to do a
             // /keys/changes request).
-            if (this.opts.crypto) {
+            if (this.syncOpts.crypto) {
-                await this.opts.crypto.onSyncCompleted(syncEventData);
+                await this.syncOpts.crypto.onSyncCompleted(syncEventData);
             }
 
             // keep emitting SYNCING -> SYNCING for clients who want to do bulk updates
@@ -907,8 +938,8 @@ export class SyncApi {
                 // stored sync data which means we don't have to worry that we may have missed
                 // device changes. We can also skip the delay since we're not calling this very
                 // frequently (and we don't really want to delay the sync for it).
-                if (this.opts.crypto) {
+                if (this.syncOpts.crypto) {
-                    await this.opts.crypto.saveDeviceList(0);
+                    await this.syncOpts.crypto.saveDeviceList(0);
                 }
 
                 // tell databases that everything is now in a consistent state and can be saved.
@@ -1356,7 +1387,7 @@ export class SyncApi {
                 if (limited) {
                     room.resetLiveTimeline(
                         joinObj.timeline.prev_batch,
-                        this.opts.canResetEntireTimeline!(room.roomId) ? null : syncEventData.oldSyncToken ?? null,
+                        this.syncOpts.canResetEntireTimeline!(room.roomId) ? null : syncEventData.oldSyncToken ?? null,
                     );
 
                     // We have to assume any gap in any timeline is
@@ -1370,10 +1401,10 @@ export class SyncApi {
             // avoids a race condition if the application tries to send a message after the
             // state event is processed, but before crypto is enabled, which then causes the
             // crypto layer to complain.
-            if (this.opts.crypto) {
+            if (this.syncOpts.crypto) {
                 for (const e of stateEvents.concat(events)) {
                     if (e.isState() && e.getType() === EventType.RoomEncryption && e.getStateKey() === "") {
-                        await this.opts.crypto.onCryptoEvent(room, e);
+                        await this.syncOpts.crypto.onCryptoEvent(room, e);
                     }
                 }
             }
@@ -1462,8 +1493,8 @@ export class SyncApi {
 
         // Handle device list updates
         if (data.device_lists) {
-            if (this.opts.crypto) {
+            if (this.syncOpts.crypto) {
-                await this.opts.crypto.handleDeviceListChanges(syncEventData, data.device_lists);
+                await this.syncOpts.crypto.handleDeviceListChanges(syncEventData, data.device_lists);
             } else {
                 // FIXME if we *don't* have a crypto module, we still need to
                 // invalidate the device lists. But that would require a
@@ -1472,12 +1503,12 @@ export class SyncApi {
         }
 
         // Handle one_time_keys_count
-        if (this.opts.crypto && data.device_one_time_keys_count) {
+        if (this.syncOpts.crypto && data.device_one_time_keys_count) {
             const currentCount = data.device_one_time_keys_count.signed_curve25519 || 0;
-            this.opts.crypto.updateOneTimeKeyCount(currentCount);
+            this.syncOpts.crypto.updateOneTimeKeyCount(currentCount);
         }
         if (
-            this.opts.crypto &&
+            this.syncOpts.crypto &&
             (data.device_unused_fallback_key_types || data["org.matrix.msc2732.device_unused_fallback_key_types"])
         ) {
             // The presence of device_unused_fallback_key_types indicates that the
@@ -1485,7 +1516,7 @@ export class SyncApi {
             // signed_curve25519 fallback key we need a new one.
             const unusedFallbackKeys =
                 data.device_unused_fallback_key_types || data["org.matrix.msc2732.device_unused_fallback_key_types"];
-            this.opts.crypto.setNeedsNewFallback(
+            this.syncOpts.crypto.setNeedsNewFallback(
                 Array.isArray(unusedFallbackKeys) && !unusedFallbackKeys.includes("signed_curve25519"),
             );
         }