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.

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);
             }

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");
     }

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);
             }

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]);
             }
         }

src/utils/arrays.ts

@@ -46,6 +46,28 @@ export function arrayDiff<T>(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<T>(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<T>(...a: T[][]): T[] {
+    return Array.from(a.reduce((c, v) => {
+        v.forEach(i => c.add(i));
+        return c;
+    }, new Set<T>()));
+}
+
 /**
  * Helper functions to perform LINQ-like queries on arrays.
  */

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);
+}