From 5df693205143f6ae0e5de84abe68b0ae67eefc3e Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Tue, 1 Dec 2020 13:19:51 -0700 Subject: [PATCH] Add various amounts of sanity checking for widget pinning This should also help https://github.com/vector-im/element-web/issues/15705 by either implicitly fixing the problem, causing chaos as described in the issue, or by forcing a crash to identify the problem more easily. --- src/stores/WidgetStore.ts | 42 +++++++++++++++++++++++++++++++++++---- 1 file changed, 38 insertions(+), 4 deletions(-) diff --git a/src/stores/WidgetStore.ts b/src/stores/WidgetStore.ts index 42a230d53a..8e08fc016c 100644 --- a/src/stores/WidgetStore.ts +++ b/src/stores/WidgetStore.ts @@ -39,9 +39,11 @@ export interface IApp extends IWidget { avatar_url: string; // MSC2765 https://github.com/matrix-org/matrix-doc/pull/2765 } +type PinnedWidgets = Record; + interface IRoomWidgets { widgets: IApp[]; - pinned: Record; + pinned: PinnedWidgets; } export const MAX_PINNED = 3; @@ -51,8 +53,9 @@ export const MAX_PINNED = 3; export default class WidgetStore extends AsyncStoreWithClient { private static internalInstance = new WidgetStore(); - private widgetMap = new Map(); - private roomMap = new Map(); + // TODO: Come up with a unique key for widgets as their IDs are not globally unique, but can exist anywhere + private widgetMap = new Map(); // Key is widget ID + private roomMap = new Map(); // Key is room ID private constructor() { super(defaultDispatcher, {}); @@ -132,6 +135,15 @@ export default class WidgetStore extends AsyncStoreWithClient { }); this.generateApps(room).forEach(app => { + // Sanity check for https://github.com/vector-im/element-web/issues/15705 + const existingApp = this.widgetMap.get(app.id); + if (existingApp) { + console.warn( + `Possible widget ID conflict for ${app.id} - wants to store in room ${app.roomId} ` + + `but is currently stored as ${existingApp.roomId} - letting the want win`, + ); + } + this.widgetMap.set(app.id, app); roomInfo.widgets.push(app); }); @@ -149,6 +161,13 @@ export default class WidgetStore extends AsyncStoreWithClient { public getRoomId = (widgetId: string) => { const app = this.widgetMap.get(widgetId); if (!app) return null; + + // Sanity check for https://github.com/vector-im/element-web/issues/15705 + const roomInfo = this.getRoom(app.roomId); + if (!roomInfo.widgets?.some(w => w.id === app.id)) { + throw new Error(`Widget ${app.id} says it is in ${app.roomId} but was not found there`); + } + return app.roomId; } @@ -158,7 +177,22 @@ export default class WidgetStore extends AsyncStoreWithClient { private onPinnedWidgetsChange = (settingName: string, roomId: string) => { this.initRoom(roomId); - this.getRoom(roomId).pinned = SettingsStore.getValue(settingName, roomId); + + const pinned: PinnedWidgets = SettingsStore.getValue(settingName, roomId); + + // Sanity check for https://github.com/vector-im/element-web/issues/15705 + const roomInfo = this.getRoom(roomId); + const remappedPinned: PinnedWidgets = {}; + for (const widgetId of Object.keys(pinned)) { + const isPinned = pinned[widgetId]; + if (!roomInfo.widgets?.some(w => w.id === widgetId)) { + console.warn(`Skipping pinned widget update for ${widgetId} in ${roomId} -- wrong room`); + } else { + remappedPinned[widgetId] = isPinned; + } + } + roomInfo.pinned = remappedPinned; + this.emit(roomId); this.emit(UPDATE_EVENT); };