Merge pull request #2705 from matrix-org/travis/fix-monitored-settings

Use a global WatchManager for settings
This commit is contained in:
Travis Ralston 2019-02-27 08:02:27 -07:00 committed by GitHub
commit 2c2685a3ea
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
13 changed files with 208 additions and 201 deletions

View file

@ -27,6 +27,7 @@ import SdkConfig from "../SdkConfig";
import dis from '../dispatcher';
import {SETTINGS} from "./Settings";
import LocalEchoWrapper from "./handlers/LocalEchoWrapper";
import {WatchManager} from "./WatchManager";
/**
* Represents the various setting levels supported by the SettingsStore.
@ -43,6 +44,8 @@ export const SettingLevel = {
DEFAULT: "default",
};
const defaultWatchManager = new WatchManager();
// Convert the settings to easier to manage objects for the handlers
const defaultSettings = {};
const invertedDefaultSettings = {};
@ -58,11 +61,11 @@ for (const key of Object.keys(SETTINGS)) {
}
const LEVEL_HANDLERS = {
"device": new DeviceSettingsHandler(featureNames),
"room-device": new RoomDeviceSettingsHandler(),
"room-account": new RoomAccountSettingsHandler(),
"account": new AccountSettingsHandler(),
"room": new RoomSettingsHandler(),
"device": new DeviceSettingsHandler(featureNames, defaultWatchManager),
"room-device": new RoomDeviceSettingsHandler(defaultWatchManager),
"room-account": new RoomAccountSettingsHandler(defaultWatchManager),
"account": new AccountSettingsHandler(defaultWatchManager),
"room": new RoomSettingsHandler(defaultWatchManager),
"config": new ConfigSettingsHandler(),
"default": new DefaultSettingsHandler(defaultSettings, invertedDefaultSettings),
};
@ -100,32 +103,30 @@ const LEVEL_ORDER = [
* be enabled).
*/
export default class SettingsStore {
// We support watching settings for changes, and do so only at the levels which are
// relevant to the setting. We pass the watcher on to the handlers and aggregate it
// before sending it off to the caller. We need to track which callback functions we
// provide to the handlers though so we can unwatch it on demand. In practice, we
// return a "callback reference" to the caller which correlates to an entry in this
// dictionary for each handler's callback function.
// We support watching settings for changes, and do this by tracking which callbacks have
// been given to us. We end up returning the callbackRef to the caller so they can unsubscribe
// at a later point.
//
// We also maintain a list of monitors which are special watchers: they cause dispatches
// when the setting changes. We track which rooms we're monitoring though to ensure we
// don't duplicate updates on the bus.
static _watchers = {}; // { callbackRef => { level => callbackFn } }
static _watchers = {}; // { callbackRef => { callbackFn } }
static _monitors = {}; // { settingName => { roomId => callbackRef } }
/**
* Watches for changes in a particular setting. This is done without any local echo
* wrapping and fires whenever a change is detected in a setting's value. Watching
* is intended to be used in scenarios where the app needs to react to changes made
* by other devices. It is otherwise expected that callers will be able to use the
* Controller system or track their own changes to settings. Callers should retain
* wrapping and fires whenever a change is detected in a setting's value, at any level.
* Watching is intended to be used in scenarios where the app needs to react to changes
* made by other devices. It is otherwise expected that callers will be able to use the
* Controller system or track their own changes to settings. Callers should retain the
* returned reference to later unsubscribe from updates.
* @param {string} settingName The setting name to watch
* @param {String} roomId The room ID to watch for changes in. May be null for 'all'.
* @param {function} callbackFn A function to be called when a setting change is
* detected. Four arguments can be expected: the setting name, the room ID (may be null),
* the level the change happened at, and finally the new value for those arguments. The
* callback may need to do a call to #getValue() to see if a consequential change has
* occurred.
* detected. Five arguments can be expected: the setting name, the room ID (may be null),
* the level the change happened at, the new value at the given level, and finally the new
* value for the setting regardless of level. The callback is responsible for determining
* if the change in value is worthwhile enough to react upon.
* @returns {string} A reference to the watcher that was employed.
*/
static watchSetting(settingName, roomId, callbackFn) {
@ -138,21 +139,15 @@ export default class SettingsStore {
}
const watcherId = `${new Date().getTime()}_${settingName}_${roomId}`;
SettingsStore._watchers[watcherId] = {};
const levels = Object.keys(LEVEL_HANDLERS);
for (const level of levels) {
const handler = SettingsStore._getHandler(originalSettingName, level);
if (!handler) continue;
const localizedCallback = (changedInRoomId, atLevel, newValAtLevel) => {
const newValue = SettingsStore.getValue(originalSettingName);
callbackFn(originalSettingName, changedInRoomId, atLevel, newValAtLevel, newValue);
};
const localizedCallback = (changedInRoomId, newVal) => {
callbackFn(originalSettingName, changedInRoomId, level, newVal);
};
console.log(`Starting watcher for ${settingName}@${roomId || '<null room>'} at level ${level}`);
SettingsStore._watchers[watcherId][level] = localizedCallback;
handler.watchSetting(settingName, roomId, localizedCallback);
}
console.log(`Starting watcher for ${settingName}@${roomId || '<null room>'}`);
SettingsStore._watchers[watcherId] = localizedCallback;
defaultWatchManager.watchSetting(settingName, roomId, localizedCallback);
return watcherId;
}
@ -166,12 +161,7 @@ export default class SettingsStore {
static unwatchSetting(watcherReference) {
if (!SettingsStore._watchers[watcherReference]) return;
for (const handlerName of Object.keys(SettingsStore._watchers[watcherReference])) {
const handler = LEVEL_HANDLERS[handlerName];
if (!handler) continue;
handler.unwatchSetting(SettingsStore._watchers[watcherReference][handlerName]);
}
defaultWatchManager.unwatchSetting(SettingsStore._watchers[watcherReference]);
delete SettingsStore._watchers[watcherReference];
}
@ -179,7 +169,7 @@ export default class SettingsStore {
* Sets up a monitor for a setting. This behaves similar to #watchSetting except instead
* of making a call to a callback, it forwards all changes to the dispatcher. Callers can
* expect to listen for the 'setting_updated' action with an object containing settingName,
* roomId, level, and newValue.
* roomId, level, newValueAtLevel, and newValue.
* @param {string} settingName The setting name to monitor.
* @param {String} roomId The room ID to monitor for changes in. Use null for all rooms.
*/
@ -188,12 +178,13 @@ export default class SettingsStore {
const registerWatcher = () => {
this._monitors[settingName][roomId] = SettingsStore.watchSetting(
settingName, roomId, (settingName, inRoomId, level, newValue) => {
settingName, roomId, (settingName, inRoomId, level, newValueAtLevel, newValue) => {
dis.dispatch({
action: 'setting_updated',
settingName,
roomId: inRoomId,
level,
newValueAtLevel,
newValue,
});
},

View file

@ -41,7 +41,11 @@ export class WatchManager {
}
}
notifyUpdate(settingName, inRoomId, newValue) {
notifyUpdate(settingName, inRoomId, atLevel, newValueAtLevel) {
// Dev note: We could avoid raising changes for ultimately inconsequential changes, but
// we also don't have a reliable way to get the old value of a setting. Instead, we'll just
// let it fall through regardless and let the receiver dedupe if they want to.
if (!this._watchers[settingName]) return;
const roomWatchers = this._watchers[settingName];
@ -51,7 +55,7 @@ export class WatchManager {
if (roomWatchers[null]) callbacks.push(...roomWatchers[null]);
for (const callback of callbacks) {
callback(inRoomId, newValue);
callback(inRoomId, atLevel, newValueAtLevel);
}
}
}

View file

@ -16,18 +16,18 @@ limitations under the License.
*/
import MatrixClientPeg from '../../MatrixClientPeg';
import {WatchManager} from "../WatchManager";
import MatrixClientBackedSettingsHandler from "./MatrixClientBackedSettingsHandler";
import {SettingLevel} from "../SettingsStore";
/**
* Gets and sets settings at the "account" level for the current user.
* This handler does not make use of the roomId parameter.
*/
export default class AccountSettingsHandler extends MatrixClientBackedSettingsHandler {
constructor() {
constructor(watchManager) {
super();
this._watchers = new WatchManager();
this._watchers = watchManager;
this._onAccountData = this._onAccountData.bind(this);
}
@ -48,11 +48,12 @@ export default class AccountSettingsHandler extends MatrixClientBackedSettingsHa
val = !val;
}
this._watchers.notifyUpdate("urlPreviewsEnabled", null, val);
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())) {
this._watchers.notifyUpdate(settingName, null, event.getContent()[settingName]);
const val = event.getContent()[settingName];
this._watchers.notifyUpdate(settingName, null, SettingLevel.ACCOUNT, val);
}
}
}
@ -102,14 +103,6 @@ export default class AccountSettingsHandler extends MatrixClientBackedSettingsHa
return cli !== undefined && cli !== null;
}
watchSetting(settingName, roomId, cb) {
this._watchers.watchSetting(settingName, roomId, cb);
}
unwatchSetting(cb) {
this._watchers.unwatchSetting(cb);
}
_getSettings(eventType = "im.vector.web.settings") {
const cli = MatrixClientPeg.get();
if (!cli) return null;

View file

@ -47,12 +47,4 @@ export default class ConfigSettingsHandler extends SettingsHandler {
isSupported() {
return true; // SdkConfig is always there
}
watchSetting(settingName, roomId, cb) {
// no-op: no changes possible
}
unwatchSetting(cb) {
// no-op: no changes possible
}
}

View file

@ -52,12 +52,4 @@ export default class DefaultSettingsHandler extends SettingsHandler {
isSupported() {
return true;
}
watchSetting(settingName, roomId, cb) {
// no-op: no changes possible
}
unwatchSetting(cb) {
// no-op: no changes possible
}
}

View file

@ -18,7 +18,7 @@ limitations under the License.
import Promise from 'bluebird';
import SettingsHandler from "./SettingsHandler";
import MatrixClientPeg from "../../MatrixClientPeg";
import {WatchManager} from "../WatchManager";
import {SettingLevel} from "../SettingsStore";
/**
* Gets and sets settings at the "device" level for the current device.
@ -29,11 +29,12 @@ export default class DeviceSettingsHandler extends SettingsHandler {
/**
* Creates a new device settings handler
* @param {string[]} featureNames The names of known features.
* @param {WatchManager} watchManager The watch manager to notify updates to
*/
constructor(featureNames) {
constructor(featureNames, watchManager) {
super();
this._featureNames = featureNames;
this._watchers = new WatchManager();
this._watchers = watchManager;
}
getValue(settingName, roomId) {
@ -69,22 +70,22 @@ export default class DeviceSettingsHandler extends SettingsHandler {
// Special case notifications
if (settingName === "notificationsEnabled") {
localStorage.setItem("notifications_enabled", newValue);
this._watchers.notifyUpdate(settingName, null, newValue);
this._watchers.notifyUpdate(settingName, null, SettingLevel.DEVICE, newValue);
return Promise.resolve();
} else if (settingName === "notificationBodyEnabled") {
localStorage.setItem("notifications_body_enabled", newValue);
this._watchers.notifyUpdate(settingName, null, newValue);
this._watchers.notifyUpdate(settingName, null, SettingLevel.DEVICE, newValue);
return Promise.resolve();
} else if (settingName === "audioNotificationsEnabled") {
localStorage.setItem("audio_notifications_enabled", newValue);
this._watchers.notifyUpdate(settingName, null, newValue);
this._watchers.notifyUpdate(settingName, null, SettingLevel.DEVICE, newValue);
return Promise.resolve();
}
const settings = this._getSettings() || {};
settings[settingName] = newValue;
localStorage.setItem("mx_local_settings", JSON.stringify(settings));
this._watchers.notifyUpdate(settingName, null, newValue);
this._watchers.notifyUpdate(settingName, null, SettingLevel.DEVICE, newValue);
return Promise.resolve();
}

View file

@ -67,12 +67,4 @@ export default class LocalEchoWrapper extends SettingsHandler {
isSupported() {
return this._handler.isSupported();
}
watchSetting(settingName, roomId, cb) {
this._handler.watchSetting(settingName, roomId, cb);
}
unwatchSetting(cb) {
this._handler.unwatchSetting(cb);
}
}

View file

@ -17,16 +17,16 @@ limitations under the License.
import MatrixClientPeg from '../../MatrixClientPeg';
import MatrixClientBackedSettingsHandler from "./MatrixClientBackedSettingsHandler";
import {WatchManager} from "../WatchManager";
import {SettingLevel} from "../SettingsStore";
/**
* Gets and sets settings at the "room-account" level for the current user.
*/
export default class RoomAccountSettingsHandler extends MatrixClientBackedSettingsHandler {
constructor() {
constructor(watchManager) {
super();
this._watchers = new WatchManager();
this._watchers = watchManager;
this._onAccountData = this._onAccountData.bind(this);
}
@ -49,13 +49,14 @@ export default class RoomAccountSettingsHandler extends MatrixClientBackedSettin
val = !val;
}
this._watchers.notifyUpdate("urlPreviewsEnabled", roomId, val);
this._watchers.notifyUpdate("urlPreviewsEnabled", roomId, SettingLevel.ROOM_ACCOUNT, val);
} else if (event.getType() === "org.matrix.room.color_scheme") {
this._watchers.notifyUpdate("roomColor", roomId, event.getContent());
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())) {
this._watchers.notifyUpdate(settingName, roomId, event.getContent()[settingName]);
const val = event.getContent()[settingName];
this._watchers.notifyUpdate(settingName, roomId, SettingLevel.ROOM_ACCOUNT, val);
}
}
}
@ -113,14 +114,6 @@ export default class RoomAccountSettingsHandler extends MatrixClientBackedSettin
return cli !== undefined && cli !== null;
}
watchSetting(settingName, roomId, cb) {
this._watchers.watchSetting(settingName, roomId, cb);
}
unwatchSetting(cb) {
this._watchers.unwatchSetting(cb);
}
_getSettings(roomId, eventType = "im.vector.web.settings") {
const room = MatrixClientPeg.get().getRoom(roomId);
if (!room) return null;

View file

@ -17,17 +17,17 @@ limitations under the License.
import Promise from 'bluebird';
import SettingsHandler from "./SettingsHandler";
import {WatchManager} from "../WatchManager";
import {SettingLevel} from "../SettingsStore";
/**
* Gets and sets settings at the "room-device" level for the current device in a particular
* room.
*/
export default class RoomDeviceSettingsHandler extends SettingsHandler {
constructor() {
constructor(watchManager) {
super();
this._watchers = new WatchManager();
this._watchers = watchManager;
}
getValue(settingName, roomId) {
@ -52,7 +52,7 @@ export default class RoomDeviceSettingsHandler extends SettingsHandler {
if (!value["blacklistUnverifiedDevicesPerRoom"]) value["blacklistUnverifiedDevicesPerRoom"] = {};
value["blacklistUnverifiedDevicesPerRoom"][roomId] = newValue;
localStorage.setItem("mx_local_settings", JSON.stringify(value));
this._watchers.notifyUpdate(settingName, roomId, newValue);
this._watchers.notifyUpdate(settingName, roomId, SettingLevel.ROOM_DEVICE, newValue);
return Promise.resolve();
}
@ -63,7 +63,7 @@ export default class RoomDeviceSettingsHandler extends SettingsHandler {
localStorage.setItem(this._getKey(settingName, roomId), newValue);
}
this._watchers.notifyUpdate(settingName, roomId, newValue);
this._watchers.notifyUpdate(settingName, roomId, SettingLevel.ROOM_DEVICE, newValue);
return Promise.resolve();
}
@ -75,14 +75,6 @@ export default class RoomDeviceSettingsHandler extends SettingsHandler {
return localStorage !== undefined && localStorage !== null;
}
watchSetting(settingName, roomId, cb) {
this._watchers.watchSetting(settingName, roomId, cb);
}
unwatchSetting(cb) {
this._watchers.unwatchSetting(cb);
}
_read(key) {
const rawValue = localStorage.getItem(key);
if (!rawValue) return null;

View file

@ -17,16 +17,16 @@ limitations under the License.
import MatrixClientPeg from '../../MatrixClientPeg';
import MatrixClientBackedSettingsHandler from "./MatrixClientBackedSettingsHandler";
import {WatchManager} from "../WatchManager";
import {SettingLevel} from "../SettingsStore";
/**
* Gets and sets settings at the "room" level.
*/
export default class RoomSettingsHandler extends MatrixClientBackedSettingsHandler {
constructor() {
constructor(watchManager) {
super();
this._watchers = new WatchManager();
this._watchers = watchManager;
this._onEvent = this._onEvent.bind(this);
}
@ -49,11 +49,11 @@ export default class RoomSettingsHandler extends MatrixClientBackedSettingsHandl
val = !val;
}
this._watchers.notifyUpdate("urlPreviewsEnabled", roomId, val);
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())) {
this._watchers.notifyUpdate(settingName, roomId, event.getContent()[settingName]);
this._watchers.notifyUpdate(settingName, roomId, SettingLevel.ROOM, event.getContent()[settingName]);
}
}
}
@ -101,14 +101,6 @@ export default class RoomSettingsHandler extends MatrixClientBackedSettingsHandl
return cli !== undefined && cli !== null;
}
watchSetting(settingName, roomId, cb) {
this._watchers.watchSetting(settingName, roomId, cb);
}
unwatchSetting(cb) {
this._watchers.unwatchSetting(cb);
}
_getSettings(roomId, eventType = "im.vector.web.settings") {
const room = MatrixClientPeg.get().getRoom(roomId);
if (!room) return null;

View file

@ -69,27 +69,4 @@ export default class SettingsHandler {
isSupported() {
return false;
}
/**
* Watches for a setting change within this handler. The caller should preserve
* a reference to the callback so that it may be unwatched. The caller should
* additionally provide a unique callback for multiple watchers on the same setting.
* @param {string} settingName The setting name to watch for changes in.
* @param {String} roomId The room ID to watch for changes in.
* @param {function} cb A function taking two arguments: the room ID the setting changed
* in and the new value for the setting at this level in the given room.
*/
watchSetting(settingName, roomId, cb) {
throw new Error("Invalid operation: watchSetting was not overridden");
}
/**
* Unwatches a previously watched setting. If the callback is not associated with
* a watcher, this is a no-op.
* @param {function} cb A callback function previously supplied to watchSetting
* which should no longer be used.
*/
unwatchSetting(cb) {
throw new Error("Invalid operation: unwatchSetting was not overridden");
}
}

View file

@ -46,6 +46,20 @@ const LIST_ORDERS = {
"im.vector.fake.archived": "recent",
};
/**
* Identifier for the "breadcrumb" (or "sort by most important room first") algorithm.
* Includes a provision for keeping the currently open room from flying down the room
* list.
* @type {string}
*/
const ALGO_IMPORTANCE = "importance";
/**
* Identifier for classic sorting behaviour: sort by the most recent message first.
* @type {string}
*/
const ALGO_RECENT = "recent";
/**
* A class for storing application state for categorising rooms in
* the RoomList.
@ -60,19 +74,18 @@ class RoomListStore extends Store {
}
/**
* Alerts the RoomListStore to a potential change in how room list sorting should
* behave.
* @param {boolean} forceRegeneration True to force a change in the algorithm
* Changes the sorting algorithm used by the RoomListStore.
* @param {string} algorithm The new algorithm to use. Should be one of the ALGO_* constants.
*/
updateSortingAlgorithm(forceRegeneration = false) {
const byImportance = SettingsStore.getValue("RoomList.orderByImportance");
if (byImportance !== this._state.orderRoomsByImportance || forceRegeneration) {
console.log("Updating room sorting algorithm: sortByImportance=" + byImportance);
this._setState({orderRoomsByImportance: byImportance});
updateSortingAlgorithm(algorithm) {
// Dev note: We only have two algorithms at the moment, but it isn't impossible that we want
// multiple in the future. Also constants make things slightly clearer.
const byImportance = algorithm === ALGO_IMPORTANCE;
console.log("Updating room sorting algorithm: sortByImportance=" + byImportance);
this._setState({orderRoomsByImportance: byImportance});
// Trigger a resort of the entire list to reflect the change in algorithm
this._generateInitialRoomLists();
}
// Trigger a resort of the entire list to reflect the change in algorithm
this._generateInitialRoomLists();
}
_init() {
@ -97,6 +110,7 @@ class RoomListStore extends Store {
};
SettingsStore.monitorSetting('RoomList.orderByImportance', null);
SettingsStore.monitorSetting('feature_custom_tags', null);
}
_setState(newState) {
@ -120,13 +134,10 @@ class RoomListStore extends Store {
switch (payload.action) {
case 'setting_updated': {
if (payload.settingName === 'RoomList.orderByImportance') {
this.updateSortingAlgorithm();
this.updateSortingAlgorithm(payload.newValue === true ? ALGO_IMPORTANCE : ALGO_RECENT);
} else if (payload.settingName === 'feature_custom_tags') {
const isActive = SettingsStore.isFeatureEnabled("feature_custom_tags");
if (isActive !== this._state.tagsEnabled) {
this._setState({tagsEnabled: isActive});
this.updateSortingAlgorithm(/*force=*/true);
}
this._setState({tagsEnabled: payload.newValue});
this._generateInitialRoomLists(); // Tags means we have to start from scratch
}
}
break;
@ -139,7 +150,10 @@ class RoomListStore extends Store {
this._setState({tagsEnabled: SettingsStore.isFeatureEnabled("feature_custom_tags")});
this._matrixClient = payload.matrixClient;
this.updateSortingAlgorithm(/*force=*/true);
const algorithm = SettingsStore.getValue("RoomList.orderByImportance")
? ALGO_IMPORTANCE : ALGO_RECENT;
this.updateSortingAlgorithm(algorithm);
}
break;
case 'MatrixActions.Room.receipt': {
@ -449,6 +463,11 @@ class RoomListStore extends Store {
}
_generateInitialRoomLists() {
// Log something to show that we're throwing away the old results. This is for the inevitable
// question of "why is 100% of my CPU going towards Riot?" - a quick look at the logs would reveal
// that something is wrong with the RoomListStore.
console.log("Generating initial room lists");
const lists = {
"m.server_notice": [],
"im.vector.fake.invite": [],