From 9b372d23ca0e09ba1d778331bed30c5d9fd5bf60 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Tue, 3 Jan 2023 13:38:21 +0000 Subject: [PATCH] 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 | 2 - spec/integ/sliding-sync-sdk.spec.ts | 7 +- src/client.ts | 55 +++++++------- src/embedded.ts | 4 +- src/sliding-sync-sdk.ts | 39 +++++----- src/sync.ts | 91 ++++++++++++++++-------- 6 files changed, 116 insertions(+), 82 deletions(-) diff --git a/spec/integ/matrix-client-methods.spec.ts b/spec/integ/matrix-client-methods.spec.ts index c83ada2d5..c1c4c30dc 100644 --- a/spec/integ/matrix-client-methods.spec.ts +++ b/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(); diff --git a/spec/integ/sliding-sync-sdk.spec.ts b/spec/integ/sliding-sync-sdk.spec.ts index 165e31420..499bf08c6 100644 --- a/spec/integ/sliding-sync-sdk.spec.ts +++ b/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) => { 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 diff --git a/src/client.ts b/src/client.ts index 91b150c8c..278237d15 100644 --- a/src/client.ts +++ b/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 { - // 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 interface IStoredClientOpts extends IStartClientOpts {} export enum RoomVersionStability { Stable = "stable", @@ -1433,20 +1423,18 @@ export class MatrixClient extends TypedEventEmitter { - if (!this.canResetTimelineCallback) { - return false; - } - return this.canResetTimelineCallback(roomId); - }; + this.clientOpts = opts ?? {}; 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 { + 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(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 { 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 = {}, + opts?: IStoredClientOpts, + syncOpts?: SyncApiOptions, ) { - this.opts.initialSyncLimit = this.opts.initialSyncLimit ?? 8; - this.opts.resolveInvitesToProfiles = this.opts.resolveInvitesToProfiles || false; - 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; - }; - } + this.opts = defaultClientOpts(opts); + this.syncOpts = defaultSyncApiOpts(syncOpts); 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) { - extensions.push(new ExtensionE2EE(this.opts.crypto)); + if (this.syncOpts.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 => { client.emit(ClientEvent.Event, e); - if (e.isState() && e.getType() == EventType.RoomEncryption && this.opts.crypto) { - await this.opts.crypto.onCryptoEvent(room, e); + if (e.isState() && e.getType() == EventType.RoomEncryption && this.syncOpts.crypto) { + await this.syncOpts.crypto.onCryptoEvent(room, e); } }; diff --git a/src/sync.ts b/src/sync.ts index 0169ff511..d2576b32a 100644 --- a/src/sync.ts +++ b/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 & { 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 = null; private currentSyncRequest?: Promise; 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 = {}) { - this.opts.initialSyncLimit = this.opts.initialSyncLimit ?? 8; - this.opts.resolveInvitesToProfiles = this.opts.resolveInvitesToProfiles || false; - 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; - }; - } + public constructor(private readonly client: MatrixClient, opts?: IStoredClientOpts, syncOpts?: SyncApiOptions) { + this.opts = defaultClientOpts(opts); + this.syncOpts = defaultSyncApiOpts(syncOpts); 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) { - await this.opts.crypto.onSyncCompleted(syncEventData); + if (this.syncOpts.crypto) { + 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) { - await this.opts.crypto.saveDeviceList(0); + if (this.syncOpts.crypto) { + 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) { - await this.opts.crypto.handleDeviceListChanges(syncEventData, data.device_lists); + if (this.syncOpts.crypto) { + 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"), ); }