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.
This commit is contained in:
Travis Ralston 2020-12-01 13:19:51 -07:00
parent bd1de8d45b
commit 5df6932051

View file

@ -39,9 +39,11 @@ export interface IApp extends IWidget {
avatar_url: string; // MSC2765 https://github.com/matrix-org/matrix-doc/pull/2765 avatar_url: string; // MSC2765 https://github.com/matrix-org/matrix-doc/pull/2765
} }
type PinnedWidgets = Record<string, boolean>;
interface IRoomWidgets { interface IRoomWidgets {
widgets: IApp[]; widgets: IApp[];
pinned: Record<string, boolean>; pinned: PinnedWidgets;
} }
export const MAX_PINNED = 3; export const MAX_PINNED = 3;
@ -51,8 +53,9 @@ export const MAX_PINNED = 3;
export default class WidgetStore extends AsyncStoreWithClient<IState> { export default class WidgetStore extends AsyncStoreWithClient<IState> {
private static internalInstance = new WidgetStore(); private static internalInstance = new WidgetStore();
private widgetMap = new Map<string, IApp>(); // TODO: Come up with a unique key for widgets as their IDs are not globally unique, but can exist anywhere
private roomMap = new Map<string, IRoomWidgets>(); private widgetMap = new Map<string, IApp>(); // Key is widget ID
private roomMap = new Map<string, IRoomWidgets>(); // Key is room ID
private constructor() { private constructor() {
super(defaultDispatcher, {}); super(defaultDispatcher, {});
@ -132,6 +135,15 @@ export default class WidgetStore extends AsyncStoreWithClient<IState> {
}); });
this.generateApps(room).forEach(app => { 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); this.widgetMap.set(app.id, app);
roomInfo.widgets.push(app); roomInfo.widgets.push(app);
}); });
@ -149,6 +161,13 @@ export default class WidgetStore extends AsyncStoreWithClient<IState> {
public getRoomId = (widgetId: string) => { public getRoomId = (widgetId: string) => {
const app = this.widgetMap.get(widgetId); const app = this.widgetMap.get(widgetId);
if (!app) return null; 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; return app.roomId;
} }
@ -158,7 +177,22 @@ export default class WidgetStore extends AsyncStoreWithClient<IState> {
private onPinnedWidgetsChange = (settingName: string, roomId: string) => { private onPinnedWidgetsChange = (settingName: string, roomId: string) => {
this.initRoom(roomId); 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(roomId);
this.emit(UPDATE_EVENT); this.emit(UPDATE_EVENT);
}; };