Add more widget sanity checking

This is for https://github.com/vector-im/element-web/issues/15705

https://github.com/matrix-org/matrix-react-sdk/pull/5459 was unable to track down all the instances of where the issue happens, so this commit tries to do a more complete job.

Specifically, this replaces the getRoomId() function given widgets cannot reliably be referenced by widget ID in this way, and the store has been updated to handle a more unique widget ID for the store (just in case).

Further sanity checking has also been added to ensure that we are at least returning a valid result.
This commit is contained in:
Travis Ralston 2020-12-02 12:20:59 -07:00
parent a1159545c4
commit d38a6ad1be

View File

@ -29,6 +29,8 @@ import WidgetUtils from "../utils/WidgetUtils";
import {SettingLevel} from "../settings/SettingLevel";
import {WidgetType} from "../widgets/WidgetType";
import {UPDATE_EVENT} from "./AsyncStore";
import { MatrixClientPeg } from "../MatrixClientPeg";
import { arrayDiff, arrayHasDiff, arrayUnion } from "../utils/arrays";
interface IState {}
@ -48,13 +50,16 @@ interface IRoomWidgets {
export const MAX_PINNED = 3;
function widgetUid(app: IApp): string {
return `${app.roomId ?? MatrixClientPeg.get().getUserId()}::${app.id}`;
}
// TODO consolidate WidgetEchoStore into this
// TODO consolidate ActiveWidgetStore into this
export default class WidgetStore extends AsyncStoreWithClient<IState> {
private static internalInstance = new WidgetStore();
// TODO: Come up with a unique key for widgets as their IDs are not globally unique, but can exist anywhere
private widgetMap = new Map<string, IApp>(); // Key is widget ID
private widgetMap = new Map<string, IApp>(); // Key is widget Unique ID (UID)
private roomMap = new Map<string, IRoomWidgets>(); // Key is room ID
private constructor() {
@ -129,14 +134,12 @@ export default class WidgetStore extends AsyncStoreWithClient<IState> {
// first clean out old widgets from the map which originate from this room
// otherwise we are out of sync with the rest of the app with stale widget events during removal
Array.from(this.widgetMap.values()).forEach(app => {
if (app.roomId === room.roomId) {
this.widgetMap.delete(app.id);
}
this.widgetMap.delete(widgetUid(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);
const existingApp = this.widgetMap.get(widgetUid(app));
if (existingApp) {
console.warn(
`Possible widget ID conflict for ${app.id} - wants to store in room ${app.roomId} ` +
@ -144,7 +147,7 @@ export default class WidgetStore extends AsyncStoreWithClient<IState> {
);
}
this.widgetMap.set(app.id, app);
this.widgetMap.set(widgetUid(app), app);
roomInfo.widgets.push(app);
});
this.emit(room.roomId);
@ -158,19 +161,6 @@ export default class WidgetStore extends AsyncStoreWithClient<IState> {
this.emit(UPDATE_EVENT);
};
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;
}
public getRoom = (roomId: string) => {
return this.roomMap.get(roomId);
};
@ -220,7 +210,7 @@ export default class WidgetStore extends AsyncStoreWithClient<IState> {
this.setPinned(roomId, widgetId, true);
// Show the apps drawer upon the user pinning a widget
if (RoomViewStore.getRoomId() === this.getRoomId(widgetId)) {
if (RoomViewStore.getRoomId() === roomId) {
defaultDispatcher.dispatch({
action: "appsDrawer",
show: true,
@ -295,12 +285,33 @@ export default class WidgetStore extends AsyncStoreWithClient<IState> {
});
const order = Object.keys(roomInfo.pinned).filter(k => roomInfo.pinned[k]);
let apps = order.map(wId => this.widgetMap.get(wId)).filter(Boolean);
apps = apps.slice(0, priorityWidget ? MAX_PINNED - 1 : MAX_PINNED);
const apps = order
.map(wId => Array.from(this.widgetMap.values())
.find(w2 => w2.roomId === roomId && w2.id === wId))
.filter(Boolean)
.slice(0, priorityWidget ? MAX_PINNED - 1 : MAX_PINNED);
if (priorityWidget) {
apps.push(priorityWidget);
}
// Sanity check for https://github.com/vector-im/element-web/issues/15705
// We union the app IDs the above generated with the roomInfo's known widgets to
// get a list of IDs which both exist. We then diff that against the generated app
// IDs above to ensure that all of the app IDs are captured by the union with the
// room - if we grabbed a widget that wasn't part of the roomInfo's list, it wouldn't
// be in the union and thus result in a diff.
const appIds = apps.map(a => widgetUid(a));
const roomAppIds = roomInfo.widgets.map(a => widgetUid(a));
const roomAppIdsUnion = arrayUnion(appIds, roomAppIds);
const missingSomeApps = arrayHasDiff(roomAppIdsUnion, appIds);
if (missingSomeApps) {
const diff = arrayDiff(roomAppIdsUnion, appIds);
console.warn(
`${roomId} appears to have a conflict for which widgets belong to it. ` +
`Widget UIDs are: `, [...diff.added, ...diff.removed],
);
}
return apps;
}