From 476f2c91e092c77e4be4bdcdebf8a734e9f180dd Mon Sep 17 00:00:00 2001 From: Germain Date: Fri, 7 Oct 2022 08:57:05 +0100 Subject: [PATCH 01/10] Add better feature detection on MatrixClient start --- spec/unit/feature.spec.ts | 62 +++++++++++++++++++++++++++++++++++++++ src/client.ts | 8 ++++- src/feature.ts | 56 +++++++++++++++++++++++++++++++++++ 3 files changed, 125 insertions(+), 1 deletion(-) create mode 100644 spec/unit/feature.spec.ts create mode 100644 src/feature.ts diff --git a/spec/unit/feature.spec.ts b/spec/unit/feature.spec.ts new file mode 100644 index 00000000000..3b6c4a75e87 --- /dev/null +++ b/spec/unit/feature.spec.ts @@ -0,0 +1,62 @@ +/* +Copyright 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. +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 { buildFeatureSupportMap, Feature } from "../../src/feature"; + +describe("Feature detection", () => { + it("checks the matrix version", async () => { + const support = await buildFeatureSupportMap({ + versions: ["v1.3"], + unstable_features: {}, + }); + + expect(support.get(Feature.Thread)).toBe(true); + expect(support.get(Feature.ThreadUnreadNotifications)).toBe(false); + }); + + it("checks the matrix msc number", async () => { + const support = await buildFeatureSupportMap({ + versions: ["v1.2"], + unstable_features: { + "org.matrix.msc3771": true, + "org.matrix.msc3773": true, + }, + }); + expect(support.get(Feature.ThreadUnreadNotifications)).toBe(true); + }); + + it("requires two MSCs to pass", async () => { + const support = await buildFeatureSupportMap({ + versions: ["v1.2"], + unstable_features: { + "org.matrix.msc3771": false, + "org.matrix.msc3773": true, + }, + }); + expect(support.get(Feature.ThreadUnreadNotifications)).toBe(false); + }); + + it("requires two MSCs OR matrix versions to pass", async () => { + const support = await buildFeatureSupportMap({ + versions: ["v1.4"], + unstable_features: { + "org.matrix.msc3771": false, + "org.matrix.msc3773": true, + }, + }); + expect(support.get(Feature.ThreadUnreadNotifications)).toBe(true); + }); +}); diff --git a/src/client.ts b/src/client.ts index 80296c472ab..fb5dc602a61 100644 --- a/src/client.ts +++ b/src/client.ts @@ -204,6 +204,7 @@ import { MAIN_ROOM_TIMELINE } from "./models/read-receipt"; import { IgnoredInvites } from "./models/invites-ignorer"; import { UIARequest, UIAResponse } from "./@types/uia"; import { LocalNotificationSettings } from "./@types/local_notifications"; +import { buildFeatureSupportMap, Feature } from "./feature"; export type Store = IStore; @@ -528,7 +529,7 @@ export interface ITurnServer { credential: string; } -interface IServerVersions { +export interface IServerVersions { versions: string[]; unstable_features: Record; } @@ -967,6 +968,8 @@ export class MatrixClient extends TypedEventEmitter; protected canResetTimelineCallback: ResetTimelineCallback; + public canSupport = new Map(); + // The pushprocessor caches useful things, so keep one and re-use it protected pushProcessor = new PushProcessor(this); @@ -1197,6 +1200,9 @@ export class MatrixClient extends TypedEventEmitter = { + [Feature.Thread]: { + unstablePrefixes: ["org.matrix.msc3440"], + matrixVersion: "v1.3", + }, + [Feature.ThreadUnreadNotifications]: { + unstablePrefixes: ["org.matrix.msc3771", "org.matrix.msc3773"], + matrixVersion: "v1.4", + }, +}; + +export async function buildFeatureSupportMap(versions: IServerVersions): Promise> { + const supportMap = new Map(); + + for (const [feature, supportCondition] of Object.entries(featureSupportResolver)) { + const supportUnstablePrefixes = supportCondition.unstablePrefixes?.every(unstablePrefix => { + return versions.unstable_features?.[unstablePrefix] === true; + }) ?? false; + + const supportMatrixVersion = versions.versions?.includes(supportCondition.matrixVersion) ?? false; + supportMap.set( + feature as Feature, + supportUnstablePrefixes || supportMatrixVersion, + ); + } + + return supportMap; +} From 0ab369bc72e04018f3e5eb0e8ffe8105df25a17f Mon Sep 17 00:00:00 2001 From: Germain Date: Fri, 7 Oct 2022 08:57:23 +0100 Subject: [PATCH 02/10] Only set unread thread notification when server support is advertised --- src/sync.ts | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/sync.ts b/src/sync.ts index 0026831d591..b26cbbeec82 100644 --- a/src/sync.ts +++ b/src/sync.ts @@ -59,6 +59,7 @@ import { BeaconEvent } from "./models/beacon"; import { IEventsResponse } from "./@types/requests"; import { IAbortablePromise } from "./@types/partials"; import { UNREAD_THREAD_NOTIFICATIONS } from "./@types/sync"; +import { Feature } from "./feature"; const DEBUG = true; @@ -562,7 +563,11 @@ export class SyncApi { }; private buildDefaultFilter = () => { - return new Filter(this.client.credentials.userId); + const filter = new Filter(this.client.credentials.userId); + if (this.client.canSupport.get(Feature.ThreadUnreadNotifications)) { + filter.setUnreadThreadNotifications(true); + } + return filter; }; private checkLazyLoadStatus = async () => { @@ -706,10 +711,6 @@ export class SyncApi { const initialFilter = this.buildDefaultFilter(); initialFilter.setDefinition(filter.getDefinition()); initialFilter.setTimelineLimit(this.opts.initialSyncLimit); - const supportsThreadNotifications = - await this.client.doesServerSupportUnstableFeature("org.matrix.msc3773") - || await this.client.isVersionSupported("v1.4"); - initialFilter.setUnreadThreadNotifications(supportsThreadNotifications); // Use an inline filter, no point uploading it for a single usage firstSyncFilter = JSON.stringify(initialFilter.getDefinition()); } From 5da1d7f8bad51d3a410f3e96e77277922af58049 Mon Sep 17 00:00:00 2001 From: Germain Date: Fri, 7 Oct 2022 09:11:58 +0100 Subject: [PATCH 03/10] fix strict null checks --- src/feature.ts | 2 +- src/filter-component.ts | 2 +- src/filter.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/feature.ts b/src/feature.ts index 0499601aa6b..f4801d2ddb4 100644 --- a/src/feature.ts +++ b/src/feature.ts @@ -45,7 +45,7 @@ export async function buildFeatureSupportMap(versions: IServerVersions): Promise return versions.unstable_features?.[unstablePrefix] === true; }) ?? false; - const supportMatrixVersion = versions.versions?.includes(supportCondition.matrixVersion) ?? false; + const supportMatrixVersion = versions.versions?.includes(supportCondition.matrixVersion || "") ?? false; supportMap.set( feature as Feature, supportUnstablePrefixes || supportMatrixVersion, diff --git a/src/filter-component.ts b/src/filter-component.ts index 8cfbea667f3..60c2721fed5 100644 --- a/src/filter-component.ts +++ b/src/filter-component.ts @@ -73,7 +73,7 @@ export interface IFilterComponent { * @param {Object} filterJson the definition of this filter JSON, e.g. { 'contains_url': true } */ export class FilterComponent { - constructor(private filterJson: IFilterComponent, public readonly userId?: string) {} + constructor(private filterJson: IFilterComponent, public readonly userId?: string | undefined) {} /** * Checks with the filter component matches the given event diff --git a/src/filter.ts b/src/filter.ts index 0cf2d1c99e5..779ced07348 100644 --- a/src/filter.ts +++ b/src/filter.ts @@ -109,7 +109,7 @@ export class Filter { private roomFilter: FilterComponent; private roomTimelineFilter: FilterComponent; - constructor(public readonly userId: string, public filterId?: string) {} + constructor(public readonly userId: string | undefined, public filterId?: string) {} /** * Get the ID of this filter on your homeserver (if known) From 5c7f47108509aae7d79f446fb7a9bd048df656ee Mon Sep 17 00:00:00 2001 From: Germain Date: Fri, 7 Oct 2022 09:17:06 +0100 Subject: [PATCH 04/10] fix strict null checks --- src/filter-component.ts | 2 +- src/filter.ts | 4 ++-- src/store/memory.ts | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/filter-component.ts b/src/filter-component.ts index 60c2721fed5..fc6e5413144 100644 --- a/src/filter-component.ts +++ b/src/filter-component.ts @@ -73,7 +73,7 @@ export interface IFilterComponent { * @param {Object} filterJson the definition of this filter JSON, e.g. { 'contains_url': true } */ export class FilterComponent { - constructor(private filterJson: IFilterComponent, public readonly userId?: string | undefined) {} + constructor(private filterJson: IFilterComponent, public readonly userId?: string | undefined | undefined) {} /** * Checks with the filter component matches the given event diff --git a/src/filter.ts b/src/filter.ts index 779ced07348..100a0359278 100644 --- a/src/filter.ts +++ b/src/filter.ts @@ -99,7 +99,7 @@ export class Filter { * @param {Object} jsonObj * @return {Filter} */ - public static fromJson(userId: string, filterId: string, jsonObj: IFilterDefinition): Filter { + public static fromJson(userId: string | undefined | null, filterId: string, jsonObj: IFilterDefinition): Filter { const filter = new Filter(userId, filterId); filter.setDefinition(jsonObj); return filter; @@ -109,7 +109,7 @@ export class Filter { private roomFilter: FilterComponent; private roomTimelineFilter: FilterComponent; - constructor(public readonly userId: string | undefined, public filterId?: string) {} + constructor(public readonly userId: string | undefined | null, public filterId?: string) {} /** * Get the ID of this filter on your homeserver (if known) diff --git a/src/store/memory.ts b/src/store/memory.ts index 0ed43a5b5ac..683ff3c0e9e 100644 --- a/src/store/memory.ts +++ b/src/store/memory.ts @@ -227,7 +227,7 @@ export class MemoryStore implements IStore { * @param {Filter} filter */ public storeFilter(filter: Filter): void { - if (!filter) { + if (!filter || !filter.userId) { return; } if (!this.filters[filter.userId]) { From 6e6d7306f454f52c8d537d9b5a545a88bb864b54 Mon Sep 17 00:00:00 2001 From: Germain Date: Fri, 7 Oct 2022 09:19:14 +0100 Subject: [PATCH 05/10] strict null checks --- src/filter-component.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/filter-component.ts b/src/filter-component.ts index fc6e5413144..5e38238c698 100644 --- a/src/filter-component.ts +++ b/src/filter-component.ts @@ -73,7 +73,7 @@ export interface IFilterComponent { * @param {Object} filterJson the definition of this filter JSON, e.g. { 'contains_url': true } */ export class FilterComponent { - constructor(private filterJson: IFilterComponent, public readonly userId?: string | undefined | undefined) {} + constructor(private filterJson: IFilterComponent, public readonly userId?: string | undefined | null) {} /** * Checks with the filter component matches the given event From ef6e5fa23cb1448a0980ccc8b7f98830f20b085c Mon Sep 17 00:00:00 2001 From: Germain Date: Fri, 7 Oct 2022 09:29:19 +0100 Subject: [PATCH 06/10] Use optional chaining Co-authored-by: Michael Telatynski <7t3chguy@gmail.com> --- src/store/memory.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/store/memory.ts b/src/store/memory.ts index 683ff3c0e9e..b44f24ca462 100644 --- a/src/store/memory.ts +++ b/src/store/memory.ts @@ -227,7 +227,7 @@ export class MemoryStore implements IStore { * @param {Filter} filter */ public storeFilter(filter: Filter): void { - if (!filter || !filter.userId) { + if (!filter?.userId) { return; } if (!this.filters[filter.userId]) { From bfadf01b15c0db263efeb2afb0a72332fc889e3d Mon Sep 17 00:00:00 2001 From: Germain Date: Fri, 7 Oct 2022 09:37:57 +0100 Subject: [PATCH 07/10] Use unread thread notification unstable value --- spec/unit/filter.spec.ts | 3 ++- src/filter.ts | 12 +++++++++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/spec/unit/filter.spec.ts b/spec/unit/filter.spec.ts index 925915729c9..e246ec1a2b2 100644 --- a/spec/unit/filter.spec.ts +++ b/spec/unit/filter.spec.ts @@ -1,3 +1,4 @@ +import { UNREAD_THREAD_NOTIFICATIONS } from "../../src/@types/sync"; import { Filter, IFilterDefinition } from "../../src/filter"; describe("Filter", function() { @@ -50,7 +51,7 @@ describe("Filter", function() { expect(filter.getDefinition()).toEqual({ room: { timeline: { - unread_thread_notifications: true, + [UNREAD_THREAD_NOTIFICATIONS.name]: true, }, }, }); diff --git a/src/filter.ts b/src/filter.ts index 100a0359278..14565c26f1a 100644 --- a/src/filter.ts +++ b/src/filter.ts @@ -22,6 +22,7 @@ import { EventType, RelationType, } from "./@types/event"; +import { UNREAD_THREAD_NOTIFICATIONS } from "./@types/sync"; import { FilterComponent, IFilterComponent } from "./filter-component"; import { MatrixEvent } from "./models/event"; @@ -227,7 +228,16 @@ export class Filter { * @param {boolean} enabled */ public setUnreadThreadNotifications(enabled: boolean): void { - setProp(this.definition, "room.timeline.unread_thread_notifications", !!enabled); + this.definition = { + ...this.definition, + room: { + ...this.definition?.room, + timeline: { + ...this.definition?.room?.timeline, + [UNREAD_THREAD_NOTIFICATIONS.name]: !!enabled, + }, + }, + }; } setLazyLoadMembers(enabled: boolean): void { From b2d5f79debc5098ea3a23ad4d38ecc85b00b0abd Mon Sep 17 00:00:00 2001 From: Germain Date: Fri, 7 Oct 2022 10:09:58 +0100 Subject: [PATCH 08/10] Better granularity on feature support --- spec/unit/feature.spec.ts | 12 ++++++------ src/@types/sync.ts | 4 ++-- src/client.ts | 10 ++++++++-- src/feature.ts | 26 ++++++++++++++++---------- src/sync.ts | 4 ++-- 5 files changed, 34 insertions(+), 22 deletions(-) diff --git a/spec/unit/feature.spec.ts b/spec/unit/feature.spec.ts index 3b6c4a75e87..8218a1330b1 100644 --- a/spec/unit/feature.spec.ts +++ b/spec/unit/feature.spec.ts @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { buildFeatureSupportMap, Feature } from "../../src/feature"; +import { buildFeatureSupportMap, Feature, ServerSupport } from "../../src/feature"; describe("Feature detection", () => { it("checks the matrix version", async () => { @@ -23,8 +23,8 @@ describe("Feature detection", () => { unstable_features: {}, }); - expect(support.get(Feature.Thread)).toBe(true); - expect(support.get(Feature.ThreadUnreadNotifications)).toBe(false); + expect(support.get(Feature.Thread)).toBe(ServerSupport.Stable); + expect(support.get(Feature.ThreadUnreadNotifications)).toBe(ServerSupport.Unsupported); }); it("checks the matrix msc number", async () => { @@ -35,7 +35,7 @@ describe("Feature detection", () => { "org.matrix.msc3773": true, }, }); - expect(support.get(Feature.ThreadUnreadNotifications)).toBe(true); + expect(support.get(Feature.ThreadUnreadNotifications)).toBe(ServerSupport.Experimental); }); it("requires two MSCs to pass", async () => { @@ -46,7 +46,7 @@ describe("Feature detection", () => { "org.matrix.msc3773": true, }, }); - expect(support.get(Feature.ThreadUnreadNotifications)).toBe(false); + expect(support.get(Feature.ThreadUnreadNotifications)).toBe(ServerSupport.Unsupported); }); it("requires two MSCs OR matrix versions to pass", async () => { @@ -57,6 +57,6 @@ describe("Feature detection", () => { "org.matrix.msc3773": true, }, }); - expect(support.get(Feature.ThreadUnreadNotifications)).toBe(true); + expect(support.get(Feature.ThreadUnreadNotifications)).toBe(ServerSupport.Stable); }); }); diff --git a/src/@types/sync.ts b/src/@types/sync.ts index f25bbf2e497..036c542bafa 100644 --- a/src/@types/sync.ts +++ b/src/@types/sync.ts @@ -14,13 +14,13 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { UnstableValue } from "matrix-events-sdk/lib/NamespacedValue"; +import { ServerControlledNamespacedValue } from "../NamespacedValue"; /** * https://github.com/matrix-org/matrix-doc/pull/3773 * * @experimental */ -export const UNREAD_THREAD_NOTIFICATIONS = new UnstableValue( +export const UNREAD_THREAD_NOTIFICATIONS = new ServerControlledNamespacedValue( "unread_thread_notifications", "org.matrix.msc3773.unread_thread_notifications"); diff --git a/src/client.ts b/src/client.ts index fb5dc602a61..6adcf3066cf 100644 --- a/src/client.ts +++ b/src/client.ts @@ -204,7 +204,10 @@ import { MAIN_ROOM_TIMELINE } from "./models/read-receipt"; import { IgnoredInvites } from "./models/invites-ignorer"; import { UIARequest, UIAResponse } from "./@types/uia"; import { LocalNotificationSettings } from "./@types/local_notifications"; -import { buildFeatureSupportMap, Feature } from "./feature"; +import { ServerSupport } from "./feature"; +import { Feature } from "./feature"; +import { buildFeatureSupportMap } from "./feature"; +import { UNREAD_THREAD_NOTIFICATIONS } from "./@types/sync"; export type Store = IStore; @@ -968,7 +971,7 @@ export class MatrixClient extends TypedEventEmitter; protected canResetTimelineCallback: ResetTimelineCallback; - public canSupport = new Map(); + public canSupport = new Map(); // The pushprocessor caches useful things, so keep one and re-use it protected pushProcessor = new PushProcessor(this); @@ -1203,6 +1206,9 @@ export class MatrixClient extends TypedEventEmitter = { }, }; -export async function buildFeatureSupportMap(versions: IServerVersions): Promise> { - const supportMap = new Map(); - +export async function buildFeatureSupportMap(versions: IServerVersions): Promise> { + const supportMap = new Map(); for (const [feature, supportCondition] of Object.entries(featureSupportResolver)) { + const supportMatrixVersion = versions.versions?.includes(supportCondition.matrixVersion || "") ?? false; const supportUnstablePrefixes = supportCondition.unstablePrefixes?.every(unstablePrefix => { return versions.unstable_features?.[unstablePrefix] === true; }) ?? false; - - const supportMatrixVersion = versions.versions?.includes(supportCondition.matrixVersion || "") ?? false; - supportMap.set( - feature as Feature, - supportUnstablePrefixes || supportMatrixVersion, - ); + if (supportMatrixVersion) { + supportMap.set(feature as Feature, ServerSupport.Stable); + } else if (supportUnstablePrefixes) { + supportMap.set(feature as Feature, ServerSupport.Experimental); + } else { + supportMap.set(feature as Feature, ServerSupport.Unsupported); + } } - return supportMap; } diff --git a/src/sync.ts b/src/sync.ts index b26cbbeec82..cee5e7f09f4 100644 --- a/src/sync.ts +++ b/src/sync.ts @@ -59,7 +59,7 @@ import { BeaconEvent } from "./models/beacon"; import { IEventsResponse } from "./@types/requests"; import { IAbortablePromise } from "./@types/partials"; import { UNREAD_THREAD_NOTIFICATIONS } from "./@types/sync"; -import { Feature } from "./feature"; +import { Feature, ServerSupport } from "./feature"; const DEBUG = true; @@ -564,7 +564,7 @@ export class SyncApi { private buildDefaultFilter = () => { const filter = new Filter(this.client.credentials.userId); - if (this.client.canSupport.get(Feature.ThreadUnreadNotifications)) { + if (this.client.canSupport.get(Feature.ThreadUnreadNotifications) !== ServerSupport.Unsupported) { filter.setUnreadThreadNotifications(true); } return filter; From 32ac04c4328b243b1a1597ee40bb3ab55888af4c Mon Sep 17 00:00:00 2001 From: Germain Date: Fri, 7 Oct 2022 10:11:49 +0100 Subject: [PATCH 09/10] fix --- src/client.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client.ts b/src/client.ts index 6adcf3066cf..d256327818c 100644 --- a/src/client.ts +++ b/src/client.ts @@ -1206,7 +1206,7 @@ export class MatrixClient extends TypedEventEmitter Date: Fri, 7 Oct 2022 11:33:36 +0100 Subject: [PATCH 10/10] Rename experimental to unstable --- spec/unit/feature.spec.ts | 2 +- src/client.ts | 6 ++---- src/feature.ts | 4 ++-- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/spec/unit/feature.spec.ts b/spec/unit/feature.spec.ts index 8218a1330b1..97420947d56 100644 --- a/spec/unit/feature.spec.ts +++ b/spec/unit/feature.spec.ts @@ -35,7 +35,7 @@ describe("Feature detection", () => { "org.matrix.msc3773": true, }, }); - expect(support.get(Feature.ThreadUnreadNotifications)).toBe(ServerSupport.Experimental); + expect(support.get(Feature.ThreadUnreadNotifications)).toBe(ServerSupport.Unstable); }); it("requires two MSCs to pass", async () => { diff --git a/src/client.ts b/src/client.ts index d256327818c..2858cab7d1f 100644 --- a/src/client.ts +++ b/src/client.ts @@ -204,10 +204,8 @@ import { MAIN_ROOM_TIMELINE } from "./models/read-receipt"; import { IgnoredInvites } from "./models/invites-ignorer"; import { UIARequest, UIAResponse } from "./@types/uia"; import { LocalNotificationSettings } from "./@types/local_notifications"; -import { ServerSupport } from "./feature"; -import { Feature } from "./feature"; -import { buildFeatureSupportMap } from "./feature"; import { UNREAD_THREAD_NOTIFICATIONS } from "./@types/sync"; +import { buildFeatureSupportMap, Feature, ServerSupport } from "./feature"; export type Store = IStore; @@ -1207,7 +1205,7 @@ export class MatrixClient extends TypedEventEmitter