From 099661c2aacd2eb360d8dcb6ac9e24972db16bc6 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 22 Jun 2020 10:18:38 -0600 Subject: [PATCH] Only fire setting changes for changed settings Previously we were firing updates for everything, which is bad. This has an effect of causing the room list to update itself every time the user goes to toggle some account settings. --- .../handlers/AccountSettingsHandler.js | 9 ++-- .../MatrixClientBackedSettingsHandler.js | 4 ++ .../handlers/RoomAccountSettingsHandler.js | 9 ++-- src/settings/handlers/RoomSettingsHandler.js | 13 +++-- src/utils/arrays.ts | 22 +++++++++ src/utils/objects.ts | 49 +++++++++++++++++++ 6 files changed, 97 insertions(+), 9 deletions(-) create mode 100644 src/utils/objects.ts diff --git a/src/settings/handlers/AccountSettingsHandler.js b/src/settings/handlers/AccountSettingsHandler.js index fea2e92c62..7b78c39c3a 100644 --- a/src/settings/handlers/AccountSettingsHandler.js +++ b/src/settings/handlers/AccountSettingsHandler.js @@ -18,6 +18,7 @@ limitations under the License. import {MatrixClientPeg} from '../../MatrixClientPeg'; import MatrixClientBackedSettingsHandler from "./MatrixClientBackedSettingsHandler"; import {SettingLevel} from "../SettingsStore"; +import {objectKeyChanges} from "../../utils/objects"; const BREADCRUMBS_LEGACY_EVENT_TYPE = "im.vector.riot.breadcrumb_rooms"; const BREADCRUMBS_EVENT_TYPE = "im.vector.setting.breadcrumbs"; @@ -45,7 +46,7 @@ export default class AccountSettingsHandler extends MatrixClientBackedSettingsHa newClient.on("accountData", this._onAccountData); } - _onAccountData(event) { + _onAccountData(event, prevEvent) { if (event.getType() === "org.matrix.preview_urls") { let val = event.getContent()['disable']; if (typeof(val) !== "boolean") { @@ -56,8 +57,10 @@ export default class AccountSettingsHandler extends MatrixClientBackedSettingsHa this._watchers.notifyUpdate("urlPreviewsEnabled", null, SettingLevel.ACCOUNT, val); } else if (event.getType() === "im.vector.web.settings") { - // We can't really discern what changed, so trigger updates for everything - for (const settingName of Object.keys(event.getContent())) { + // Figure out what changed and fire those updates + const prevContent = prevEvent ? prevEvent.getContent() : {}; + const changedSettings = objectKeyChanges(prevContent, event.getContent()); + for (const settingName of changedSettings) { const val = event.getContent()[settingName]; this._watchers.notifyUpdate(settingName, null, SettingLevel.ACCOUNT, val); } diff --git a/src/settings/handlers/MatrixClientBackedSettingsHandler.js b/src/settings/handlers/MatrixClientBackedSettingsHandler.js index effe7ae9a7..63725b4dff 100644 --- a/src/settings/handlers/MatrixClientBackedSettingsHandler.js +++ b/src/settings/handlers/MatrixClientBackedSettingsHandler.js @@ -42,6 +42,10 @@ export default class MatrixClientBackedSettingsHandler extends SettingsHandler { MatrixClientBackedSettingsHandler._instances.push(this); } + get client() { + return MatrixClientBackedSettingsHandler._matrixClient; + } + initMatrixClient() { console.warn("initMatrixClient not overridden"); } diff --git a/src/settings/handlers/RoomAccountSettingsHandler.js b/src/settings/handlers/RoomAccountSettingsHandler.js index 1e9d3f7bed..1c818cef71 100644 --- a/src/settings/handlers/RoomAccountSettingsHandler.js +++ b/src/settings/handlers/RoomAccountSettingsHandler.js @@ -18,6 +18,7 @@ limitations under the License. import {MatrixClientPeg} from '../../MatrixClientPeg'; import MatrixClientBackedSettingsHandler from "./MatrixClientBackedSettingsHandler"; import {SettingLevel} from "../SettingsStore"; +import {objectKeyChanges} from "../../utils/objects"; const ALLOWED_WIDGETS_EVENT_TYPE = "im.vector.setting.allowed_widgets"; @@ -40,7 +41,7 @@ export default class RoomAccountSettingsHandler extends MatrixClientBackedSettin newClient.on("Room.accountData", this._onAccountData); } - _onAccountData(event, room) { + _onAccountData(event, room, prevEvent) { const roomId = room.roomId; if (event.getType() === "org.matrix.room.preview_urls") { @@ -55,8 +56,10 @@ export default class RoomAccountSettingsHandler extends MatrixClientBackedSettin } else if (event.getType() === "org.matrix.room.color_scheme") { this._watchers.notifyUpdate("roomColor", roomId, SettingLevel.ROOM_ACCOUNT, event.getContent()); } else if (event.getType() === "im.vector.web.settings") { - // We can't really discern what changed, so trigger updates for everything - for (const settingName of Object.keys(event.getContent())) { + // Figure out what changed and fire those updates + const prevContent = prevEvent ? prevEvent.getContent() : {}; + const changedSettings = objectKeyChanges(prevContent, event.getContent()); + for (const settingName of changedSettings) { const val = event.getContent()[settingName]; this._watchers.notifyUpdate(settingName, roomId, SettingLevel.ROOM_ACCOUNT, val); } diff --git a/src/settings/handlers/RoomSettingsHandler.js b/src/settings/handlers/RoomSettingsHandler.js index 6407818450..4116f26220 100644 --- a/src/settings/handlers/RoomSettingsHandler.js +++ b/src/settings/handlers/RoomSettingsHandler.js @@ -18,6 +18,7 @@ limitations under the License. import {MatrixClientPeg} from '../../MatrixClientPeg'; import MatrixClientBackedSettingsHandler from "./MatrixClientBackedSettingsHandler"; import {SettingLevel} from "../SettingsStore"; +import {objectKeyChanges} from "../../utils/objects"; /** * Gets and sets settings at the "room" level. @@ -38,8 +39,12 @@ export default class RoomSettingsHandler extends MatrixClientBackedSettingsHandl newClient.on("RoomState.events", this._onEvent); } - _onEvent(event) { + _onEvent(event, state, prevEvent) { const roomId = event.getRoomId(); + const room = this.client.getRoom(roomId); + if (!room) throw new Error(`Unknown room caused state update: ${roomId}`); + + if (state !== room.currentState) return; // ignore state updates which are not current if (event.getType() === "org.matrix.room.preview_urls") { let val = event.getContent()['disable']; @@ -51,8 +56,10 @@ export default class RoomSettingsHandler extends MatrixClientBackedSettingsHandl this._watchers.notifyUpdate("urlPreviewsEnabled", roomId, SettingLevel.ROOM, val); } else if (event.getType() === "im.vector.web.settings") { - // We can't really discern what changed, so trigger updates for everything - for (const settingName of Object.keys(event.getContent())) { + // Figure out what changed and fire those updates + const prevContent = prevEvent ? prevEvent.getContent() : {}; + const changedSettings = objectKeyChanges(prevContent, event.getContent()); + for (const settingName of changedSettings) { this._watchers.notifyUpdate(settingName, roomId, SettingLevel.ROOM, event.getContent()[settingName]); } } diff --git a/src/utils/arrays.ts b/src/utils/arrays.ts index fea376afcd..8175d89464 100644 --- a/src/utils/arrays.ts +++ b/src/utils/arrays.ts @@ -46,6 +46,28 @@ export function arrayDiff(a: T[], b: T[]): { added: T[], removed: T[] } { }; } +/** + * Returns the union of two arrays. + * @param a The first array. Must be defined. + * @param b The second array. Must be defined. + * @returns The union of the arrays. + */ +export function arrayUnion(a: T[], b: T[]): T[] { + return a.filter(i => b.includes(i)); +} + +/** + * Merges arrays, deduping contents using a Set. + * @param a The arrays to merge. + * @returns The merged array. + */ +export function arrayMerge(...a: T[][]): T[] { + return Array.from(a.reduce((c, v) => { + v.forEach(i => c.add(i)); + return c; + }, new Set())); +} + /** * Helper functions to perform LINQ-like queries on arrays. */ diff --git a/src/utils/objects.ts b/src/utils/objects.ts new file mode 100644 index 0000000000..cbb311cc48 --- /dev/null +++ b/src/utils/objects.ts @@ -0,0 +1,49 @@ +/* +Copyright 2020 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 { arrayDiff, arrayMerge, arrayUnion } from "./arrays"; + +/** + * Determines the keys added, changed, and removed between two objects. + * For changes, simple triple equal comparisons are done, not in-depth + * tree checking. + * @param a The first object. Must be defined. + * @param b The second object. Must be defined. + * @returns The difference between the keys of each object. + */ +export function objectDiff(a: any, b: any): { changed: string[], added: string[], removed: string[] } { + const aKeys = Object.keys(a); + const bKeys = Object.keys(b); + const keyDiff = arrayDiff(aKeys, bKeys); + const possibleChanges = arrayUnion(aKeys, bKeys); + const changes = possibleChanges.filter(k => a[k] !== b[k]); + + return {changed: changes, added: keyDiff.added, removed: keyDiff.removed}; +} + +/** + * Gets all the key changes (added, removed, or value difference) between + * two objects. Triple equals is used to compare values, not in-depth tree + * checking. + * @param a The first object. Must be defined. + * @param b The second object. Must be defined. + * @returns The keys which have been added, removed, or changed between the + * two objects. + */ +export function objectKeyChanges(a: any, b: any): string[] { + const diff = objectDiff(a, b); + return arrayMerge(diff.removed, diff.added, diff.changed); +}