From 64ccc868929c28c86770fd9795111075ffa2ad43 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Tue, 19 Apr 2022 17:27:32 +0100 Subject: [PATCH 1/5] Cache localStorage objects for SettingsStore --- .../AbstractLocalStorageSettingsHandler.ts | 96 +++++++++++++++++++ .../handlers/DeviceSettingsHandler.ts | 34 +++---- .../handlers/RoomDeviceSettingsHandler.ts | 23 ++--- 3 files changed, 118 insertions(+), 35 deletions(-) create mode 100644 src/settings/handlers/AbstractLocalStorageSettingsHandler.ts diff --git a/src/settings/handlers/AbstractLocalStorageSettingsHandler.ts b/src/settings/handlers/AbstractLocalStorageSettingsHandler.ts new file mode 100644 index 00000000000..f3a8877c66d --- /dev/null +++ b/src/settings/handlers/AbstractLocalStorageSettingsHandler.ts @@ -0,0 +1,96 @@ +/* +Copyright 2019 - 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 SettingsHandler from "./SettingsHandler"; + +/** + * Abstract settings handler wrapping around localStorage making getValue calls cheaper + * by caching the values and listening for localStorage updates from other tabs. + */ +export default abstract class AbstractLocalStorageSettingsHandler extends SettingsHandler { + private itemCache = new Map(); + private objectCache = new Map(); + + protected constructor() { + super(); + + // Listen for storage changes from other tabs + window.addEventListener("storage", (e: StorageEvent) => { + if (this.itemCache.has(e.key)) { + this.itemCache.set(e.key, e.newValue); + } + + if (this.objectCache.has(e.key)) { + try { + this.objectCache.set(e.key, JSON.parse(e.newValue)); + } catch (err) { + console.error("Failed to parse localStorage object", err); + this.objectCache.delete(e.key); + } + } + }); + } + + protected getItem(key: string): any { + if (!this.itemCache.has(key)) { + const value = localStorage.getItem(key); + this.itemCache.set(key, value); + return value; + } + + return this.itemCache.get(key); + } + + protected getObject(key: string): T { + if (!this.objectCache.has(key)) { + try { + const value = JSON.parse(localStorage.getItem(key)); + this.objectCache.set(key, value); + return value; + } catch (err) { + console.error("Failed to parse localStorage object", err); + this.objectCache.delete(key); + return null; + } + } + + return this.objectCache.get(key) as T; + } + + protected setItem(key: string, value: any): void { + this.itemCache.set(key, value); + localStorage.setItem(key, value); + } + + protected setObject(key: string, value: object): void { + this.itemCache.set(key, value); + localStorage.setItem(key, JSON.stringify(value)); + } + + protected removeItem(key: string): void { + localStorage.removeItem(key); + this.itemCache.delete(key); + } + + protected removeObject(key: string): void { + localStorage.removeItem(key); + this.objectCache.delete(key); + } + + public isSupported(): boolean { + return localStorage !== undefined && localStorage !== null; + } +} diff --git a/src/settings/handlers/DeviceSettingsHandler.ts b/src/settings/handlers/DeviceSettingsHandler.ts index 7d2fbaf236a..25c75c67a19 100644 --- a/src/settings/handlers/DeviceSettingsHandler.ts +++ b/src/settings/handlers/DeviceSettingsHandler.ts @@ -1,7 +1,7 @@ /* Copyright 2017 Travis Ralston Copyright 2019 New Vector Ltd. -Copyright 2019 The Matrix.org Foundation C.I.C. +Copyright 2019 - 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. @@ -16,17 +16,17 @@ See the License for the specific language governing permissions and limitations under the License. */ -import SettingsHandler from "./SettingsHandler"; import { MatrixClientPeg } from "../../MatrixClientPeg"; import { SettingLevel } from "../SettingLevel"; import { CallbackFn, WatchManager } from "../WatchManager"; +import AbstractLocalStorageSettingsHandler from "./AbstractLocalStorageSettingsHandler"; /** * Gets and sets settings at the "device" level for the current device. * This handler does not make use of the roomId parameter. This handler * will special-case features to support legacy settings. */ -export default class DeviceSettingsHandler extends SettingsHandler { +export default class DeviceSettingsHandler extends AbstractLocalStorageSettingsHandler { /** * Creates a new device settings handler * @param {string[]} featureNames The names of known features. @@ -43,15 +43,15 @@ export default class DeviceSettingsHandler extends SettingsHandler { // Special case notifications if (settingName === "notificationsEnabled") { - const value = localStorage.getItem("notifications_enabled"); + const value = this.getItem("notifications_enabled"); if (typeof(value) === "string") return value === "true"; return null; // wrong type or otherwise not set } else if (settingName === "notificationBodyEnabled") { - const value = localStorage.getItem("notifications_body_enabled"); + const value = this.getItem("notifications_body_enabled"); if (typeof(value) === "string") return value === "true"; return null; // wrong type or otherwise not set } else if (settingName === "audioNotificationsEnabled") { - const value = localStorage.getItem("audio_notifications_enabled"); + const value = this.getItem("audio_notifications_enabled"); if (typeof(value) === "string") return value === "true"; return null; // wrong type or otherwise not set } @@ -68,15 +68,15 @@ export default class DeviceSettingsHandler extends SettingsHandler { // Special case notifications if (settingName === "notificationsEnabled") { - localStorage.setItem("notifications_enabled", newValue); + this.setItem("notifications_enabled", newValue); this.watchers.notifyUpdate(settingName, null, SettingLevel.DEVICE, newValue); return Promise.resolve(); } else if (settingName === "notificationBodyEnabled") { - localStorage.setItem("notifications_body_enabled", newValue); + this.setItem("notifications_body_enabled", newValue); this.watchers.notifyUpdate(settingName, null, SettingLevel.DEVICE, newValue); return Promise.resolve(); } else if (settingName === "audioNotificationsEnabled") { - localStorage.setItem("audio_notifications_enabled", newValue); + this.setItem("audio_notifications_enabled", newValue); this.watchers.notifyUpdate(settingName, null, SettingLevel.DEVICE, newValue); return Promise.resolve(); } @@ -87,7 +87,7 @@ export default class DeviceSettingsHandler extends SettingsHandler { delete settings["useIRCLayout"]; settings["layout"] = newValue; - localStorage.setItem("mx_local_settings", JSON.stringify(settings)); + this.setObject("mx_local_settings", settings); this.watchers.notifyUpdate(settingName, null, SettingLevel.DEVICE, newValue); return Promise.resolve(); @@ -95,7 +95,7 @@ export default class DeviceSettingsHandler extends SettingsHandler { const settings = this.getSettings() || {}; settings[settingName] = newValue; - localStorage.setItem("mx_local_settings", JSON.stringify(settings)); + this.setObject("mx_local_settings", settings); this.watchers.notifyUpdate(settingName, null, SettingLevel.DEVICE, newValue); return Promise.resolve(); @@ -105,10 +105,6 @@ export default class DeviceSettingsHandler extends SettingsHandler { return true; // It's their device, so they should be able to } - public isSupported(): boolean { - return localStorage !== undefined && localStorage !== null; - } - public watchSetting(settingName: string, roomId: string, cb: CallbackFn) { this.watchers.watchSetting(settingName, roomId, cb); } @@ -118,9 +114,7 @@ export default class DeviceSettingsHandler extends SettingsHandler { } private getSettings(): any { // TODO: [TS] Type return - const value = localStorage.getItem("mx_local_settings"); - if (!value) return null; - return JSON.parse(value); + return this.getObject("mx_local_settings"); } // Note: features intentionally don't use the same key as settings to avoid conflicts @@ -132,7 +126,7 @@ export default class DeviceSettingsHandler extends SettingsHandler { return false; } - const value = localStorage.getItem("mx_labs_feature_" + featureName); + const value = this.getItem("mx_labs_feature_" + featureName); if (value === "true") return true; if (value === "false") return false; // Try to read the next config level for the feature. @@ -140,7 +134,7 @@ export default class DeviceSettingsHandler extends SettingsHandler { } private writeFeature(featureName: string, enabled: boolean | null) { - localStorage.setItem("mx_labs_feature_" + featureName, `${enabled}`); + this.setItem("mx_labs_feature_" + featureName, `${enabled}`); this.watchers.notifyUpdate(featureName, null, SettingLevel.DEVICE, enabled); } } diff --git a/src/settings/handlers/RoomDeviceSettingsHandler.ts b/src/settings/handlers/RoomDeviceSettingsHandler.ts index 47fcecdfacd..a0fdbed0e53 100644 --- a/src/settings/handlers/RoomDeviceSettingsHandler.ts +++ b/src/settings/handlers/RoomDeviceSettingsHandler.ts @@ -1,6 +1,6 @@ /* Copyright 2017 Travis Ralston -Copyright 2019, 2020 The Matrix.org Foundation C.I.C. +Copyright 2019, 2020 - 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. @@ -15,15 +15,15 @@ See the License for the specific language governing permissions and limitations under the License. */ -import SettingsHandler from "./SettingsHandler"; import { SettingLevel } from "../SettingLevel"; import { WatchManager } from "../WatchManager"; +import AbstractLocalStorageSettingsHandler from "./AbstractLocalStorageSettingsHandler"; /** * Gets and sets settings at the "room-device" level for the current device in a particular * room. */ -export default class RoomDeviceSettingsHandler extends SettingsHandler { +export default class RoomDeviceSettingsHandler extends AbstractLocalStorageSettingsHandler { constructor(public readonly watchers: WatchManager) { super(); } @@ -32,7 +32,7 @@ export default class RoomDeviceSettingsHandler extends SettingsHandler { // Special case blacklist setting to use legacy values if (settingName === "blacklistUnverifiedDevices") { const value = this.read("mx_local_settings"); - if (value && value['blacklistUnverifiedDevicesPerRoom']) { + if (value?.['blacklistUnverifiedDevicesPerRoom']) { return value['blacklistUnverifiedDevicesPerRoom'][roomId]; } } @@ -49,16 +49,15 @@ export default class RoomDeviceSettingsHandler extends SettingsHandler { if (!value) value = {}; if (!value["blacklistUnverifiedDevicesPerRoom"]) value["blacklistUnverifiedDevicesPerRoom"] = {}; value["blacklistUnverifiedDevicesPerRoom"][roomId] = newValue; - localStorage.setItem("mx_local_settings", JSON.stringify(value)); + this.setObject("mx_local_settings", value); this.watchers.notifyUpdate(settingName, roomId, SettingLevel.ROOM_DEVICE, newValue); return Promise.resolve(); } if (newValue === null) { - localStorage.removeItem(this.getKey(settingName, roomId)); + this.removeObject(this.getKey(settingName, roomId)); } else { - newValue = JSON.stringify({ value: newValue }); - localStorage.setItem(this.getKey(settingName, roomId), newValue); + this.setObject(this.getKey(settingName, roomId), { value: newValue }); } this.watchers.notifyUpdate(settingName, roomId, SettingLevel.ROOM_DEVICE, newValue); @@ -69,14 +68,8 @@ export default class RoomDeviceSettingsHandler extends SettingsHandler { return true; // It's their device, so they should be able to } - public isSupported(): boolean { - return localStorage !== undefined && localStorage !== null; - } - private read(key: string): any { - const rawValue = localStorage.getItem(key); - if (!rawValue) return null; - return JSON.parse(rawValue); + return this.getItem(key); } private getKey(settingName: string, roomId: string): string { From 2d75c93a791d9ba6b11023d6244cf1bd8ec3aa44 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Tue, 19 Apr 2022 17:29:24 +0100 Subject: [PATCH 2/5] Lazily cache-bust --- .../AbstractLocalStorageSettingsHandler.ts | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/src/settings/handlers/AbstractLocalStorageSettingsHandler.ts b/src/settings/handlers/AbstractLocalStorageSettingsHandler.ts index f3a8877c66d..e12cf7b203b 100644 --- a/src/settings/handlers/AbstractLocalStorageSettingsHandler.ts +++ b/src/settings/handlers/AbstractLocalStorageSettingsHandler.ts @@ -27,20 +27,10 @@ export default abstract class AbstractLocalStorageSettingsHandler extends Settin protected constructor() { super(); - // Listen for storage changes from other tabs + // Listen for storage changes from other tabs to bust the cache window.addEventListener("storage", (e: StorageEvent) => { - if (this.itemCache.has(e.key)) { - this.itemCache.set(e.key, e.newValue); - } - - if (this.objectCache.has(e.key)) { - try { - this.objectCache.set(e.key, JSON.parse(e.newValue)); - } catch (err) { - console.error("Failed to parse localStorage object", err); - this.objectCache.delete(e.key); - } - } + this.itemCache.delete(e.key); + this.objectCache.delete(e.key); }); } @@ -54,7 +44,7 @@ export default abstract class AbstractLocalStorageSettingsHandler extends Settin return this.itemCache.get(key); } - protected getObject(key: string): T { + protected getObject(key: string): T | null { if (!this.objectCache.has(key)) { try { const value = JSON.parse(localStorage.getItem(key)); @@ -62,7 +52,6 @@ export default abstract class AbstractLocalStorageSettingsHandler extends Settin return value; } catch (err) { console.error("Failed to parse localStorage object", err); - this.objectCache.delete(key); return null; } } From d9fef9519978fe8e061916e7aae1ccd11e45ffc3 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Tue, 19 Apr 2022 17:32:29 +0100 Subject: [PATCH 3/5] consolidate --- src/settings/handlers/AbstractLocalStorageSettingsHandler.ts | 5 +---- src/settings/handlers/RoomDeviceSettingsHandler.ts | 2 +- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/settings/handlers/AbstractLocalStorageSettingsHandler.ts b/src/settings/handlers/AbstractLocalStorageSettingsHandler.ts index e12cf7b203b..95794782a45 100644 --- a/src/settings/handlers/AbstractLocalStorageSettingsHandler.ts +++ b/src/settings/handlers/AbstractLocalStorageSettingsHandler.ts @@ -69,13 +69,10 @@ export default abstract class AbstractLocalStorageSettingsHandler extends Settin localStorage.setItem(key, JSON.stringify(value)); } + // handles both items and objects protected removeItem(key: string): void { localStorage.removeItem(key); this.itemCache.delete(key); - } - - protected removeObject(key: string): void { - localStorage.removeItem(key); this.objectCache.delete(key); } diff --git a/src/settings/handlers/RoomDeviceSettingsHandler.ts b/src/settings/handlers/RoomDeviceSettingsHandler.ts index a0fdbed0e53..c1d1b57e9b6 100644 --- a/src/settings/handlers/RoomDeviceSettingsHandler.ts +++ b/src/settings/handlers/RoomDeviceSettingsHandler.ts @@ -55,7 +55,7 @@ export default class RoomDeviceSettingsHandler extends AbstractLocalStorageSetti } if (newValue === null) { - this.removeObject(this.getKey(settingName, roomId)); + this.removeItem(this.getKey(settingName, roomId)); } else { this.setObject(this.getKey(settingName, roomId), { value: newValue }); } From d190334681aed5e8302b9ae20f7df17194f2d8a7 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Tue, 19 Apr 2022 17:58:40 +0100 Subject: [PATCH 4/5] Fix typo --- src/settings/handlers/AbstractLocalStorageSettingsHandler.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/settings/handlers/AbstractLocalStorageSettingsHandler.ts b/src/settings/handlers/AbstractLocalStorageSettingsHandler.ts index 95794782a45..902c00f7a16 100644 --- a/src/settings/handlers/AbstractLocalStorageSettingsHandler.ts +++ b/src/settings/handlers/AbstractLocalStorageSettingsHandler.ts @@ -65,7 +65,7 @@ export default abstract class AbstractLocalStorageSettingsHandler extends Settin } protected setObject(key: string, value: object): void { - this.itemCache.set(key, value); + this.objectCache.set(key, value); localStorage.setItem(key, JSON.stringify(value)); } From 904f37173860b33b46db4e657da66c15999bc544 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Wed, 20 Apr 2022 09:20:49 +0100 Subject: [PATCH 5/5] Handle StorageEvent clear properly --- .../handlers/AbstractLocalStorageSettingsHandler.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/settings/handlers/AbstractLocalStorageSettingsHandler.ts b/src/settings/handlers/AbstractLocalStorageSettingsHandler.ts index 902c00f7a16..5d64009b6fe 100644 --- a/src/settings/handlers/AbstractLocalStorageSettingsHandler.ts +++ b/src/settings/handlers/AbstractLocalStorageSettingsHandler.ts @@ -29,8 +29,13 @@ export default abstract class AbstractLocalStorageSettingsHandler extends Settin // Listen for storage changes from other tabs to bust the cache window.addEventListener("storage", (e: StorageEvent) => { - this.itemCache.delete(e.key); - this.objectCache.delete(e.key); + if (e.key === null) { + this.itemCache.clear(); + this.objectCache.clear(); + } else { + this.itemCache.delete(e.key); + this.objectCache.delete(e.key); + } }); }