From 819fc75202f21e9bb51014c9844ddc6769dc8d99 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 19 Jun 2024 11:24:56 +0100 Subject: [PATCH] Fetch capabilities in the background (#4246) * Fetch capabilities in the background & keep them up to date * Add missed await * Replace some more runAllTimers and round down the wait time for sanity * Remove double comment * Typo * Add a method back that will fetch capabilities if they're not already there * Add tests * Catch exception here too * Add test for room version code --- spec/integ/crypto/megolm-backup.spec.ts | 4 +- spec/integ/matrix-client-methods.spec.ts | 107 ++++++++++++-- .../matrix-client-syncing-errors.spec.ts | 9 +- spec/test-utils/client.ts | 2 +- spec/unit/rendezvous/rendezvous.spec.ts | 2 +- spec/unit/room.spec.ts | 43 ++++++ src/client.ts | 110 +++++--------- src/matrix.ts | 1 + src/models/room.ts | 16 ++- src/rendezvous/MSC3906Rendezvous.ts | 9 +- src/serverCapabilities.ts | 136 ++++++++++++++++++ 11 files changed, 340 insertions(+), 99 deletions(-) create mode 100644 src/serverCapabilities.ts diff --git a/spec/integ/crypto/megolm-backup.spec.ts b/spec/integ/crypto/megolm-backup.spec.ts index eee0eafebc2..bc36f940ffc 100644 --- a/spec/integ/crypto/megolm-backup.spec.ts +++ b/spec/integ/crypto/megolm-backup.spec.ts @@ -796,7 +796,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("megolm-keys backup (%s)", (backe const result = await aliceCrypto.checkKeyBackupAndEnable(); expect(result).toBeTruthy(); - jest.runAllTimers(); + jest.advanceTimersByTime(10 * 60 * 1000); await failurePromise; // Fix the endpoint to do successful uploads @@ -829,7 +829,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("megolm-keys backup (%s)", (backe }); // run the timers, which will make the backup loop redo the request - await jest.runAllTimersAsync(); + await jest.advanceTimersByTimeAsync(10 * 60 * 1000); await successPromise; await allKeysUploadedPromise; }); diff --git a/spec/integ/matrix-client-methods.spec.ts b/spec/integ/matrix-client-methods.spec.ts index c321403d591..72793711f62 100644 --- a/spec/integ/matrix-client-methods.spec.ts +++ b/spec/integ/matrix-client-methods.spec.ts @@ -1293,18 +1293,109 @@ describe("MatrixClient", function () { }); describe("getCapabilities", () => { - it("should cache by default", async () => { + it("should return cached capabilities if present", async () => { + const capsObject = { + "m.change_password": false, + }; + + httpBackend!.when("GET", "/versions").respond(200, {}); + httpBackend!.when("GET", "/pushrules").respond(200, {}); + httpBackend!.when("POST", "/filter").respond(200, { filter_id: "a filter id" }); httpBackend.when("GET", "/capabilities").respond(200, { - capabilities: { - "m.change_password": false, - }, + capabilities: capsObject, + }); + + client.startClient(); + await httpBackend!.flushAllExpected(); + + expect(await client.getCapabilities()).toEqual(capsObject); + }); + + it("should fetch capabilities if cache not present", async () => { + const capsObject = { + "m.change_password": false, + }; + + httpBackend.when("GET", "/capabilities").respond(200, { + capabilities: capsObject, + }); + + const capsPromise = client.getCapabilities(); + await httpBackend!.flushAllExpected(); + + expect(await capsPromise).toEqual(capsObject); + }); + }); + + describe("getCachedCapabilities", () => { + it("should return cached capabilities or undefined", async () => { + const capsObject = { + "m.change_password": false, + }; + + httpBackend!.when("GET", "/versions").respond(200, {}); + httpBackend!.when("GET", "/pushrules").respond(200, {}); + httpBackend!.when("POST", "/filter").respond(200, { filter_id: "a filter id" }); + httpBackend.when("GET", "/capabilities").respond(200, { + capabilities: capsObject, + }); + + expect(client.getCachedCapabilities()).toBeUndefined(); + + client.startClient(); + + await httpBackend!.flushAllExpected(); + + expect(client.getCachedCapabilities()).toEqual(capsObject); + }); + }); + + describe("fetchCapabilities", () => { + const capsObject = { + "m.change_password": false, + }; + + beforeEach(() => { + httpBackend.when("GET", "/capabilities").respond(200, { + capabilities: capsObject, }); - const prom = httpBackend.flushAllExpected(); - const capabilities1 = await client.getCapabilities(); - const capabilities2 = await client.getCapabilities(); + }); + + afterEach(() => { + jest.useRealTimers(); + }); + + it("should always fetch capabilities and then cache", async () => { + const prom = client.fetchCapabilities(); + await httpBackend.flushAllExpected(); + const caps = await prom; + + expect(caps).toEqual(capsObject); + }); + + it("should write-through the cache", async () => { + httpBackend!.when("GET", "/versions").respond(200, {}); + httpBackend!.when("GET", "/pushrules").respond(200, {}); + httpBackend!.when("POST", "/filter").respond(200, { filter_id: "a filter id" }); + + client.startClient(); + await httpBackend!.flushAllExpected(); + + expect(client.getCachedCapabilities()).toEqual(capsObject); + + const newCapsObject = { + "m.change_password": true, + }; + + httpBackend.when("GET", "/capabilities").respond(200, { + capabilities: newCapsObject, + }); + + const prom = client.fetchCapabilities(); + await httpBackend.flushAllExpected(); await prom; - expect(capabilities1).toStrictEqual(capabilities2); + expect(client.getCachedCapabilities()).toEqual(newCapsObject); }); }); diff --git a/spec/integ/matrix-client-syncing-errors.spec.ts b/spec/integ/matrix-client-syncing-errors.spec.ts index ab11a33adfc..4133ffb9aaa 100644 --- a/spec/integ/matrix-client-syncing-errors.spec.ts +++ b/spec/integ/matrix-client-syncing-errors.spec.ts @@ -105,13 +105,13 @@ describe("MatrixClient syncing errors", () => { await client!.startClient(); expect(await syncEvents[0].promise).toBe(SyncState.Error); - jest.runAllTimers(); // this will skip forward to trigger the keepAlive/sync + jest.advanceTimersByTime(60 * 1000); // this will skip forward to trigger the keepAlive/sync expect(await syncEvents[1].promise).toBe(SyncState.Error); - jest.runAllTimers(); // this will skip forward to trigger the keepAlive/sync + jest.advanceTimersByTime(60 * 1000); // this will skip forward to trigger the keepAlive/sync expect(await syncEvents[2].promise).toBe(SyncState.Prepared); - jest.runAllTimers(); // this will skip forward to trigger the keepAlive/sync + jest.advanceTimersByTime(60 * 1000); // this will skip forward to trigger the keepAlive/sync expect(await syncEvents[3].promise).toBe(SyncState.Syncing); - jest.runAllTimers(); // this will skip forward to trigger the keepAlive/sync + jest.advanceTimersByTime(60 * 1000); // this will skip forward to trigger the keepAlive/sync expect(await syncEvents[4].promise).toBe(SyncState.Syncing); }); @@ -119,6 +119,7 @@ describe("MatrixClient syncing errors", () => { jest.useFakeTimers(); fetchMock.config.overwriteRoutes = false; fetchMock + .get("end:capabilities", {}) .getOnce("end:versions", {}) // first version check without credentials needs to succeed .get("end:versions", unknownTokenErrorData) // further version checks fails with 401 .get("end:pushrules/", 401) // fails with 401 without an error. This does happen in practice e.g. with Synapse diff --git a/spec/test-utils/client.ts b/spec/test-utils/client.ts index 8123d0d65e6..62e11dffa13 100644 --- a/spec/test-utils/client.ts +++ b/spec/test-utils/client.ts @@ -88,6 +88,6 @@ export const mockClientMethodsEvents = () => ({ export const mockClientMethodsServer = (): Partial, unknown>> => ({ getIdentityServerUrl: jest.fn(), getHomeserverUrl: jest.fn(), - getCapabilities: jest.fn().mockReturnValue({}), + getCachedCapabilities: jest.fn().mockReturnValue({}), doesServerSupportUnstableFeature: jest.fn().mockResolvedValue(false), }); diff --git a/spec/unit/rendezvous/rendezvous.spec.ts b/spec/unit/rendezvous/rendezvous.spec.ts index f600639d829..a208ffc590e 100644 --- a/spec/unit/rendezvous/rendezvous.spec.ts +++ b/spec/unit/rendezvous/rendezvous.spec.ts @@ -116,7 +116,7 @@ function makeMockClient(opts: { }, }; }, - getCapabilities() { + getCachedCapabilities() { return opts.msc3882r0Only ? {} : { diff --git a/spec/unit/room.spec.ts b/spec/unit/room.spec.ts index eb90b8bdfa0..542bbcb2536 100644 --- a/spec/unit/room.spec.ts +++ b/spec/unit/room.spec.ts @@ -4034,4 +4034,47 @@ describe("Room", function () { expect(room.getLastThread()).toBe(thread2); }); }); + + describe("getRecommendedVersion", () => { + it("returns the server's recommended version from capabilities", async () => { + const client = new TestClient(userA).client; + client.getCapabilities = jest.fn().mockReturnValue({ + ["m.room_versions"]: { + default: "1", + available: ["1", "2"], + }, + }); + const room = new Room(roomId, client, userA); + expect(await room.getRecommendedVersion()).toEqual({ + version: "1", + needsUpgrade: false, + urgent: false, + }); + }); + + it("force-refreshes versions to make sure an upgrade is necessary", async () => { + const client = new TestClient(userA).client; + client.getCapabilities = jest.fn().mockReturnValue({ + ["m.room_versions"]: { + default: "5", + available: ["5"], + }, + }); + + client.fetchCapabilities = jest.fn().mockResolvedValue({ + ["m.room_versions"]: { + default: "1", + available: ["1"], + }, + }); + + const room = new Room(roomId, client, userA); + expect(await room.getRecommendedVersion()).toEqual({ + version: "1", + needsUpgrade: false, + urgent: false, + }); + expect(client.fetchCapabilities).toHaveBeenCalled(); + }); + }); }); diff --git a/src/client.ts b/src/client.ts index 40a9e5d89b7..300b5903399 100644 --- a/src/client.ts +++ b/src/client.ts @@ -226,6 +226,7 @@ import { getRelationsThreadFilter } from "./thread-utils"; import { KnownMembership, Membership } from "./@types/membership"; import { RoomMessageEventContent, StickerEventContent } from "./@types/events"; import { ImageInfo } from "./@types/media"; +import { Capabilities, ServerCapabilities } from "./serverCapabilities"; export type Store = IStore; @@ -233,7 +234,6 @@ export type ResetTimelineCallback = (roomId: string) => boolean; const SCROLLBACK_DELAY_MS = 3000; export const CRYPTO_ENABLED: boolean = isCryptoAvailable(); -const CAPABILITIES_CACHE_MS = 21600000; // 6 hours - an arbitrary value const TURN_CHECK_INTERVAL = 10 * 60 * 1000; // poll for turn credentials every 10 minutes export const UNSTABLE_MSC3852_LAST_SEEN_UA = new UnstableValue( @@ -518,26 +518,6 @@ export interface IStartClientOpts { export interface IStoredClientOpts extends IStartClientOpts {} -export enum RoomVersionStability { - Stable = "stable", - Unstable = "unstable", -} - -export interface IRoomVersionsCapability { - default: string; - available: Record; -} - -export interface ICapability { - enabled: boolean; -} - -export interface IChangePasswordCapability extends ICapability {} - -export interface IThreadsCapability extends ICapability {} - -export interface IGetLoginTokenCapability extends ICapability {} - export const GET_LOGIN_TOKEN_CAPABILITY = new NamespacedValue( "m.get_login_token", "org.matrix.msc3882.get_login_token", @@ -547,19 +527,6 @@ export const UNSTABLE_MSC2666_SHARED_ROOMS = "uk.half-shot.msc2666"; export const UNSTABLE_MSC2666_MUTUAL_ROOMS = "uk.half-shot.msc2666.mutual_rooms"; export const UNSTABLE_MSC2666_QUERY_MUTUAL_ROOMS = "uk.half-shot.msc2666.query_mutual_rooms"; -/** - * A representation of the capabilities advertised by a homeserver as defined by - * [Capabilities negotiation](https://spec.matrix.org/v1.6/client-server-api/#get_matrixclientv3capabilities). - */ -export interface Capabilities { - [key: string]: any; - "m.change_password"?: IChangePasswordCapability; - "m.room_versions"?: IRoomVersionsCapability; - "io.element.thread"?: IThreadsCapability; - "m.get_login_token"?: IGetLoginTokenCapability; - "org.matrix.msc3882.get_login_token"?: IGetLoginTokenCapability; -} - enum CrossSigningKeyType { MasterKey = "master_key", SelfSigningKey = "self_signing_key", @@ -1293,10 +1260,6 @@ export class MatrixClient extends TypedEventEmitter; - public cachedCapabilities?: { - capabilities: Capabilities; - expiration: number; - }; protected clientWellKnown?: IClientWellKnown; protected clientWellKnownPromise?: Promise; protected turnServers: ITurnServer[] = []; @@ -1325,6 +1288,8 @@ export class MatrixClient extends TypedEventEmitter { - const now = new Date().getTime(); - - if (this.cachedCapabilities && !fresh) { - if (now < this.cachedCapabilities.expiration) { - this.logger.debug("Returning cached capabilities"); - return Promise.resolve(this.cachedCapabilities.capabilities); - } - } - - type Response = { - capabilities?: Capabilities; - }; - return this.http - .authedRequest(Method.Get, "/capabilities") - .catch((e: Error): Response => { - // We swallow errors because we need a default object anyhow - this.logger.error(e); - return {}; - }) - .then((r = {}) => { - const capabilities = r["capabilities"] || {}; - - // If the capabilities missed the cache, cache it for a shorter amount - // of time to try and refresh them later. - const cacheMs = Object.keys(capabilities).length ? CAPABILITIES_CACHE_MS : 60000 + Math.random() * 5000; + public async getCapabilities(): Promise { + const caps = this.serverCapabilitiesService.getCachedCapabilities(); + if (caps) return caps; + return this.serverCapabilitiesService.fetchCapabilities(); + } - this.cachedCapabilities = { - capabilities, - expiration: now + cacheMs, - }; + /** + * Gets the cached capabilities of the homeserver. If none have been fetched yet, + * return undefined. + * + * @returns The capabilities of the homeserver + */ + public getCachedCapabilities(): Capabilities | undefined { + return this.serverCapabilitiesService.getCachedCapabilities(); + } - this.logger.debug("Caching capabilities: ", capabilities); - return capabilities; - }); + /** + * Fetches the latest capabilities from the homeserver, ignoring any cached + * versions. The newly returned version is cached. + * + * @returns A promise which resolves to the capabilities of the homeserver + */ + public fetchCapabilities(): Promise { + return this.serverCapabilitiesService.fetchCapabilities(); } /** diff --git a/src/matrix.ts b/src/matrix.ts index 793a894e0d0..50a2a72f65d 100644 --- a/src/matrix.ts +++ b/src/matrix.ts @@ -24,6 +24,7 @@ import { RoomWidgetClient, ICapabilities } from "./embedded"; import { CryptoStore } from "./crypto/store/base"; export * from "./client"; +export * from "./serverCapabilities"; export * from "./embedded"; export * from "./http-api"; export * from "./autodiscovery"; diff --git a/src/models/room.ts b/src/models/room.ts index d8b90a60856..7e1284b4e9b 100644 --- a/src/models/room.ts +++ b/src/models/room.ts @@ -41,7 +41,7 @@ import { RelationType, UNSIGNED_THREAD_ID_FIELD, } from "../@types/event"; -import { IRoomVersionsCapability, MatrixClient, PendingEventOrdering, RoomVersionStability } from "../client"; +import { MatrixClient, PendingEventOrdering } from "../client"; import { GuestAccess, HistoryVisibility, JoinRule, ResizeMethod } from "../@types/partials"; import { Filter, IFilterDefinition } from "../filter"; import { RoomState, RoomStateEvent, RoomStateEventHandlerMap } from "./room-state"; @@ -70,6 +70,7 @@ import { RoomReceipts } from "./room-receipts"; import { compareEventOrdering } from "./compare-event-ordering"; import * as utils from "../utils"; import { KnownMembership, Membership } from "../@types/membership"; +import { Capabilities, IRoomVersionsCapability, RoomVersionStability } from "../serverCapabilities"; // These constants are used as sane defaults when the homeserver doesn't support // the m.room_versions capability. In practice, KNOWN_SAFE_ROOM_VERSION should be @@ -611,7 +612,10 @@ export class Room extends ReadReceipt { * Resolves to the version the room should be upgraded to. */ public async getRecommendedVersion(): Promise { - const capabilities = await this.client.getCapabilities(); + let capabilities: Capabilities = {}; + try { + capabilities = await this.client.getCapabilities(); + } catch (e) {} let versionCap = capabilities["m.room_versions"]; if (!versionCap) { versionCap = { @@ -636,8 +640,12 @@ export class Room extends ReadReceipt { "to be supporting a newer room version we don't know about.", ); - const caps = await this.client.getCapabilities(true); - versionCap = caps["m.room_versions"]; + try { + capabilities = await this.client.fetchCapabilities(); + } catch (e) { + logger.warn("Failed to refresh room version capabilities", e); + } + versionCap = capabilities["m.room_versions"]; if (!versionCap) { logger.warn("No room version capability - assuming upgrade required."); return result; diff --git a/src/rendezvous/MSC3906Rendezvous.ts b/src/rendezvous/MSC3906Rendezvous.ts index f83aff26123..4a2c3fa3319 100644 --- a/src/rendezvous/MSC3906Rendezvous.ts +++ b/src/rendezvous/MSC3906Rendezvous.ts @@ -22,12 +22,12 @@ import { LegacyRendezvousFailureReason as RendezvousFailureReason, RendezvousIntent, } from "."; -import { IGetLoginTokenCapability, MatrixClient, GET_LOGIN_TOKEN_CAPABILITY } from "../client"; +import { MatrixClient, GET_LOGIN_TOKEN_CAPABILITY } from "../client"; import { buildFeatureSupportMap, Feature, ServerSupport } from "../feature"; import { logger } from "../logger"; import { sleep } from "../utils"; import { CrossSigningKey } from "../crypto-api"; -import { Device } from "../matrix"; +import { Capabilities, Device, IGetLoginTokenCapability } from "../matrix"; enum PayloadType { Start = "m.login.start", @@ -109,7 +109,10 @@ export class MSC3906Rendezvous { logger.info(`Connected to secure channel with checksum: ${checksum} our intent is ${this.ourIntent}`); // in stable and unstable r1 the availability is exposed as a capability - const capabilities = await this.client.getCapabilities(); + let capabilities: Capabilities = {}; + try { + capabilities = await this.client.getCapabilities(); + } catch (e) {} // in r0 of MSC3882 the availability is exposed as a feature flag const features = await buildFeatureSupportMap(await this.client.getVersions()); const capability = GET_LOGIN_TOKEN_CAPABILITY.findIn(capabilities); diff --git a/src/serverCapabilities.ts b/src/serverCapabilities.ts new file mode 100644 index 00000000000..845d2e9ac72 --- /dev/null +++ b/src/serverCapabilities.ts @@ -0,0 +1,136 @@ +/* +Copyright 2024 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. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import { IHttpOpts, MatrixHttpApi, Method } from "./http-api"; +import { logger } from "./logger"; + +// How often we update the server capabilities. +// 6 hours - an arbitrary value, but they should change very infrequently. +const CAPABILITIES_CACHE_MS = 6 * 60 * 60 * 1000; + +// How long we want before retrying if we couldn't fetch +const CAPABILITIES_RETRY_MS = 30 * 1000; + +export interface ICapability { + enabled: boolean; +} + +export interface IChangePasswordCapability extends ICapability {} + +export interface IThreadsCapability extends ICapability {} + +export interface IGetLoginTokenCapability extends ICapability {} + +export interface ISetDisplayNameCapability extends ICapability {} + +export interface ISetAvatarUrlCapability extends ICapability {} + +export enum RoomVersionStability { + Stable = "stable", + Unstable = "unstable", +} + +export interface IRoomVersionsCapability { + default: string; + available: Record; +} + +/** + * A representation of the capabilities advertised by a homeserver as defined by + * [Capabilities negotiation](https://spec.matrix.org/v1.6/client-server-api/#get_matrixclientv3capabilities). + */ +export interface Capabilities { + [key: string]: any; + "m.change_password"?: IChangePasswordCapability; + "m.room_versions"?: IRoomVersionsCapability; + "io.element.thread"?: IThreadsCapability; + "m.get_login_token"?: IGetLoginTokenCapability; + "org.matrix.msc3882.get_login_token"?: IGetLoginTokenCapability; + "m.set_displayname"?: ISetDisplayNameCapability; + "m.set_avatar_url"?: ISetAvatarUrlCapability; +} + +type CapabilitiesResponse = { + capabilities: Capabilities; +}; + +/** + * Manages storing and periodically refreshing the server capabilities. + */ +export class ServerCapabilities { + private capabilities?: Capabilities; + private retryTimeout?: ReturnType; + private refreshTimeout?: ReturnType; + + public constructor(private readonly http: MatrixHttpApi) {} + + /** + * Starts periodically fetching the server capabilities. + */ + public start(): void { + this.poll().then(); + } + + /** + * Stops the service + */ + public stop(): void { + this.clearTimeouts(); + } + + /** + * Returns the cached capabilities, or undefined if none are cached. + * @returns the current capabilities, if any. + */ + public getCachedCapabilities(): Capabilities | undefined { + return this.capabilities; + } + + /** + * Fetches the latest server capabilities from the homeserver and returns them, or rejects + * on failure. + */ + public fetchCapabilities = async (): Promise => { + const resp = await this.http.authedRequest(Method.Get, "/capabilities"); + this.capabilities = resp["capabilities"]; + return this.capabilities; + }; + + private poll = async (): Promise => { + try { + await this.fetchCapabilities(); + this.clearTimeouts(); + this.refreshTimeout = setTimeout(this.poll, CAPABILITIES_CACHE_MS); + logger.debug("Fetched new server capabilities"); + } catch (e) { + this.clearTimeouts(); + const howLong = Math.floor(CAPABILITIES_RETRY_MS + Math.random() * 5000); + this.retryTimeout = setTimeout(this.poll, howLong); + logger.warn(`Failed to refresh capabilities: retrying in ${howLong}ms`, e); + } + }; + + private clearTimeouts(): void { + if (this.refreshTimeout) { + clearInterval(this.refreshTimeout); + this.refreshTimeout = undefined; + } + if (this.retryTimeout) { + clearTimeout(this.retryTimeout); + this.retryTimeout = undefined; + } + } +}