From 1cb794753e4114c86132de7058d13a920fafe448 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 13 Jun 2018 10:39:52 +0100 Subject: [PATCH 1/4] Simplify & refactor some widget stuff * ScalarMessaging onMessage was getting the current room ID by listening for view_and remembering the room id or alias, and so having to look up the alias if it was alias. We have RoomViewStore for this. * Move waitForUserWidget into WidgetUtils * s/require/import/ --- src/ScalarMessaging.js | 193 +++++++++++++---------------------------- src/WidgetUtils.js | 46 ++++++++++ 2 files changed, 106 insertions(+), 133 deletions(-) diff --git a/src/ScalarMessaging.js b/src/ScalarMessaging.js index 9457e6ccfb..dd3975dfe5 100644 --- a/src/ScalarMessaging.js +++ b/src/ScalarMessaging.js @@ -1,6 +1,7 @@ /* Copyright 2016 OpenMarket Ltd Copyright 2017 Vector Creations Ltd +Copyright 2018 New Vector Ltd Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -231,11 +232,12 @@ Example: } */ -const SdkConfig = require('./SdkConfig'); -const MatrixClientPeg = require("./MatrixClientPeg"); -const MatrixEvent = require("matrix-js-sdk").MatrixEvent; -const dis = require("./dispatcher"); -const Widgets = require('./utils/widgets'); +import SdkConfig from './SdkConfig'; +import MatrixClientPeg from './MatrixClientPeg'; +import { MatrixEvent } from 'matrix-js-sdk'; +import dis from './dispatcher'; +import Widgets from './utils/widgets'; +import RoomViewStore from './stores/RoomViewStore'; import { _t } from './languageHandler'; function sendResponse(event, res) { @@ -286,51 +288,6 @@ function inviteUser(event, roomId, userId) { }); } -/** - * Returns a promise that resolves when a widget with the given - * ID has been added as a user widget (ie. the accountData event - * arrives) or rejects after a timeout - * - * @param {string} widgetId The ID of the widget to wait for - * @param {boolean} add True to wait for the widget to be added, - * false to wait for it to be deleted. - * @returns {Promise} that resolves when the widget is available - */ -function waitForUserWidget(widgetId, add) { - return new Promise((resolve, reject) => { - const currentAccountDataEvent = MatrixClientPeg.get().getAccountData('m.widgets'); - - // Tests an account data event, returning true if it's in the state - // we're waiting for it to be in - function eventInIntendedState(ev) { - if (!ev || !currentAccountDataEvent.getContent()) return false; - if (add) { - return ev.getContent()[widgetId] !== undefined; - } else { - return ev.getContent()[widgetId] === undefined; - } - } - - if (eventInIntendedState(currentAccountDataEvent)) { - resolve(); - return; - } - - function onAccountData(ev) { - if (eventInIntendedState(currentAccountDataEvent)) { - MatrixClientPeg.get().removeListener('accountData', onAccountData); - clearTimeout(timerId); - resolve(); - } - } - const timerId = setTimeout(() => { - MatrixClientPeg.get().removeListener('accountData', onAccountData); - reject(new Error("Timed out waiting for widget ID " + widgetId + " to appear")); - }, 10000); - MatrixClientPeg.get().on('accountData', onAccountData); - }); -} - function setWidget(event, roomId) { const widgetId = event.data.widget_id; const widgetType = event.data.type; @@ -637,19 +594,6 @@ function returnStateEvent(event, roomId, eventType, stateKey) { sendResponse(event, stateEvent.getContent()); } -let currentRoomId = null; -let currentRoomAlias = null; - -// Listen for when a room is viewed -dis.register(onAction); -function onAction(payload) { - if (payload.action !== "view_room") { - return; - } - currentRoomId = payload.room_id; - currentRoomAlias = payload.room_alias; -} - const onMessage = function(event) { if (!event.origin) { // stupid chrome event.origin = event.originalEvent.origin; @@ -700,80 +644,63 @@ const onMessage = function(event) { return; } } - let promise = Promise.resolve(currentRoomId); - if (!currentRoomId) { - if (!currentRoomAlias) { - sendError(event, _t('Must be viewing a room')); - return; - } - // no room ID but there is an alias, look it up. - console.log("Looking up alias " + currentRoomAlias); - promise = MatrixClientPeg.get().getRoomIdForAlias(currentRoomAlias).then((res) => { - return res.room_id; - }); + + if (roomId !== RoomViewStore.getRoomId()) { + sendError(event, _t('Room %(roomId)s not visible', {roomId: roomId})); + return; } - promise.then((viewingRoomId) => { - if (roomId !== viewingRoomId) { - sendError(event, _t('Room %(roomId)s not visible', {roomId: roomId})); - return; - } + // Get and set room-based widgets + if (event.data.action === "get_widgets") { + getWidgets(event, roomId); + return; + } else if (event.data.action === "set_widget") { + setWidget(event, roomId); + return; + } - // Get and set room-based widgets - if (event.data.action === "get_widgets") { - getWidgets(event, roomId); - return; - } else if (event.data.action === "set_widget") { - setWidget(event, roomId); - return; - } + // These APIs don't require userId + if (event.data.action === "join_rules_state") { + getJoinRules(event, roomId); + return; + } else if (event.data.action === "set_plumbing_state") { + setPlumbingState(event, roomId, event.data.status); + return; + } else if (event.data.action === "get_membership_count") { + getMembershipCount(event, roomId); + return; + } else if (event.data.action === "get_room_enc_state") { + getRoomEncState(event, roomId); + return; + } else if (event.data.action === "can_send_event") { + canSendEvent(event, roomId); + return; + } - // These APIs don't require userId - if (event.data.action === "join_rules_state") { - getJoinRules(event, roomId); - return; - } else if (event.data.action === "set_plumbing_state") { - setPlumbingState(event, roomId, event.data.status); - return; - } else if (event.data.action === "get_membership_count") { - getMembershipCount(event, roomId); - return; - } else if (event.data.action === "get_room_enc_state") { - getRoomEncState(event, roomId); - return; - } else if (event.data.action === "can_send_event") { - canSendEvent(event, roomId); - return; - } - - if (!userId) { - sendError(event, _t('Missing user_id in request')); - return; - } - switch (event.data.action) { - case "membership_state": - getMembershipState(event, roomId, userId); - break; - case "invite": - inviteUser(event, roomId, userId); - break; - case "bot_options": - botOptions(event, roomId, userId); - break; - case "set_bot_options": - setBotOptions(event, roomId, userId); - break; - case "set_bot_power": - setBotPower(event, roomId, userId, event.data.level); - break; - default: - console.warn("Unhandled postMessage event with action '" + event.data.action +"'"); - break; - } - }, (err) => { - console.error(err); - sendError(event, _t('Failed to lookup current room') + '.'); - }); + if (!userId) { + sendError(event, _t('Missing user_id in request')); + return; + } + switch (event.data.action) { + case "membership_state": + getMembershipState(event, roomId, userId); + break; + case "invite": + inviteUser(event, roomId, userId); + break; + case "bot_options": + botOptions(event, roomId, userId); + break; + case "set_bot_options": + setBotOptions(event, roomId, userId); + break; + case "set_bot_power": + setBotPower(event, roomId, userId, event.data.level); + break; + default: + console.warn("Unhandled postMessage event with action '" + event.data.action +"'"); + break; + } }; let listenerCount = 0; diff --git a/src/WidgetUtils.js b/src/WidgetUtils.js index 10cd473904..c6816d28b6 100644 --- a/src/WidgetUtils.js +++ b/src/WidgetUtils.js @@ -1,5 +1,6 @@ /* Copyright 2017 Vector Creations Ltd +Copyright 2018 New Vector Ltd Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -90,4 +91,49 @@ export default class WidgetUtils { } return false; } + + /** + * Returns a promise that resolves when a widget with the given + * ID has been added as a user widget (ie. the accountData event + * arrives) or rejects after a timeout + * + * @param {string} widgetId The ID of the widget to wait for + * @param {boolean} add True to wait for the widget to be added, + * false to wait for it to be deleted. + * @returns {Promise} that resolves when the widget is available + */ + static waitForUserWidget(widgetId, add) { + return new Promise((resolve, reject) => { + const currentAccountDataEvent = MatrixClientPeg.get().getAccountData('m.widgets'); + + // Tests an account data event, returning true if it's in the state + // we're waiting for it to be in + function eventInIntendedState(ev) { + if (!ev || !currentAccountDataEvent.getContent()) return false; + if (add) { + return ev.getContent()[widgetId] !== undefined; + } else { + return ev.getContent()[widgetId] === undefined; + } + } + + if (eventInIntendedState(currentAccountDataEvent)) { + resolve(); + return; + } + + function onAccountData(ev) { + if (eventInIntendedState(currentAccountDataEvent)) { + MatrixClientPeg.get().removeListener('accountData', onAccountData); + clearTimeout(timerId); + resolve(); + } + } + const timerId = setTimeout(() => { + MatrixClientPeg.get().removeListener('accountData', onAccountData); + reject(new Error("Timed out waiting for widget ID " + widgetId + " to appear")); + }, 10000); + MatrixClientPeg.get().on('accountData', onAccountData); + }); + } } From 94125fb5668150a64c173356ee94a30798e6ecc8 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 13 Jun 2018 15:50:19 +0100 Subject: [PATCH 2/4] Fix widgets re-appearing after being deleted Widgets would sometimes briefly re-appear after having been deleted. This was because of the following race: * User presses delete, send POST req, we set `deleting`. Widget hides. * POST request completes, we unset `deleting` so widget unhides. * State event comes down sync so widget hides again. This fixes this by introducing `waitForRoomWidget` and using it to wait until the state event comes down the sync until clearing the `deleting` flag. Since we now have `waitForRoomWidget`, this also uses it when adding a widget so the 'widget saved' appears at the same time the widget does. --- src/ScalarMessaging.js | 9 ++-- src/WidgetUtils.js | 60 ++++++++++++++++++++++-- src/components/views/elements/AppTile.js | 4 +- 3 files changed, 64 insertions(+), 9 deletions(-) diff --git a/src/ScalarMessaging.js b/src/ScalarMessaging.js index dd3975dfe5..f80162e635 100644 --- a/src/ScalarMessaging.js +++ b/src/ScalarMessaging.js @@ -237,6 +237,7 @@ import MatrixClientPeg from './MatrixClientPeg'; import { MatrixEvent } from 'matrix-js-sdk'; import dis from './dispatcher'; import Widgets from './utils/widgets'; +import WidgetUtils from './WidgetUtils'; import RoomViewStore from './stores/RoomViewStore'; import { _t } from './languageHandler'; @@ -362,7 +363,7 @@ function setWidget(event, roomId) { // wait for this, the action will complete but if the user is fast enough, // the widget still won't actually be there. client.setAccountData('m.widgets', userWidgets).then(() => { - return waitForUserWidget(widgetId, widgetUrl !== null); + return WidgetUtils.waitForUserWidget(widgetId, widgetUrl !== null); }).then(() => { sendResponse(event, { success: true, @@ -382,9 +383,9 @@ function setWidget(event, roomId) { } // TODO - Room widgets need to be moved to 'm.widget' state events // https://docs.google.com/document/d/1uPF7XWY_dXTKVKV7jZQ2KmsI19wn9-kFRgQ1tFQP7wQ/edit?usp=sharing - client.sendStateEvent(roomId, "im.vector.modular.widgets", content, widgetId).done(() => { - // XXX: We should probably wait for the echo of the state event to come back from the server, - // as we do with user widgets. + client.sendStateEvent(roomId, "im.vector.modular.widgets", content, widgetId).then(() => { + return WidgetUtils.waitForRoomWidget(widgetId, roomId, widgetUrl !== null); + }).then(() => { sendResponse(event, { success: true, }); diff --git a/src/WidgetUtils.js b/src/WidgetUtils.js index c6816d28b6..14fe3f59bd 100644 --- a/src/WidgetUtils.js +++ b/src/WidgetUtils.js @@ -104,12 +104,10 @@ export default class WidgetUtils { */ static waitForUserWidget(widgetId, add) { return new Promise((resolve, reject) => { - const currentAccountDataEvent = MatrixClientPeg.get().getAccountData('m.widgets'); - // Tests an account data event, returning true if it's in the state // we're waiting for it to be in function eventInIntendedState(ev) { - if (!ev || !currentAccountDataEvent.getContent()) return false; + if (!ev || !ev.getContent()) return false; if (add) { return ev.getContent()[widgetId] !== undefined; } else { @@ -117,12 +115,14 @@ export default class WidgetUtils { } } - if (eventInIntendedState(currentAccountDataEvent)) { + const startingAccountDataEvent = MatrixClientPeg.get().getAccountData('m.widgets'); + if (eventInIntendedState(startingAccountDataEvent)) { resolve(); return; } function onAccountData(ev) { + const currentAccountDataEvent = MatrixClientPeg.get().getAccountData('m.widgets'); if (eventInIntendedState(currentAccountDataEvent)) { MatrixClientPeg.get().removeListener('accountData', onAccountData); clearTimeout(timerId); @@ -136,4 +136,56 @@ export default class WidgetUtils { MatrixClientPeg.get().on('accountData', onAccountData); }); } + + /** + * Returns a promise that resolves when a widget with the given + * ID has been added as a room widget in the given room (ie. the + * room state event arrives) or rejects after a timeout + * + * @param {string} widgetId The ID of the widget to wait for + * @param {string} roomId The ID of the room to wait for the widget in + * @param {boolean} add True to wait for the widget to be added, + * false to wait for it to be deleted. + * @returns {Promise} that resolves when the widget is available + */ + static waitForRoomWidget(widgetId, roomId, add) { + return new Promise((resolve, reject) => { + // Tests a list of state events, returning true if it's in the state + // we're waiting for it to be in + function eventsInIntendedState(evList) { + const widgetPresent = evList.some((ev) => { + return ev.getContent() && ev.getContent()['id'] === widgetId; + }); + if (add) { + return widgetPresent; + } else { + return !widgetPresent; + } + } + + const room = MatrixClientPeg.get().getRoom(roomId); + const startingWidgetEvents = room.currentState.getStateEvents('im.vector.modular.widgets'); + if (eventsInIntendedState(startingWidgetEvents)) { + resolve(); + return; + } + + function onRoomStateEvents(ev) { + if (ev.getRoomId() !== roomId) return; + + const currentWidgetEvents = room.currentState.getStateEvents('im.vector.modular.widgets'); + + if (eventsInIntendedState(currentWidgetEvents)) { + MatrixClientPeg.get().removeListener('RoomState.events', onRoomStateEvents); + clearTimeout(timerId); + resolve(); + } + } + const timerId = setTimeout(() => { + MatrixClientPeg.get().removeListener('RoomState.events', onRoomStateEvents); + reject(new Error("Timed out waiting for widget ID " + widgetId + " to appear")); + }, 10000); + MatrixClientPeg.get().on('RoomState.events', onRoomStateEvents); + }); + } } diff --git a/src/components/views/elements/AppTile.js b/src/components/views/elements/AppTile.js index 429b5941b9..70b5bd651e 100644 --- a/src/components/views/elements/AppTile.js +++ b/src/components/views/elements/AppTile.js @@ -324,7 +324,9 @@ export default class AppTile extends React.Component { 'im.vector.modular.widgets', {}, // empty content this.props.id, - ).catch((e) => { + ).then(() => { + return WidgetUtils.waitForRoomWidget(this.props.id, this.props.room.roomId, false); + }).catch((e) => { console.error('Failed to delete widget', e); }).finally(() => { this.setState({deleting: false}); From 36574ca0fb1b0a14a5886a953b3d6dbaad2ea7d2 Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 14 Jun 2018 13:03:42 +0100 Subject: [PATCH 3/4] Fix doc --- src/WidgetUtils.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/WidgetUtils.js b/src/WidgetUtils.js index 14fe3f59bd..98ee935f35 100644 --- a/src/WidgetUtils.js +++ b/src/WidgetUtils.js @@ -100,7 +100,8 @@ export default class WidgetUtils { * @param {string} widgetId The ID of the widget to wait for * @param {boolean} add True to wait for the widget to be added, * false to wait for it to be deleted. - * @returns {Promise} that resolves when the widget is available + * @returns {Promise} that resolves when the widget is the the + * requested state according to the `add` param */ static waitForUserWidget(widgetId, add) { return new Promise((resolve, reject) => { @@ -146,7 +147,8 @@ export default class WidgetUtils { * @param {string} roomId The ID of the room to wait for the widget in * @param {boolean} add True to wait for the widget to be added, * false to wait for it to be deleted. - * @returns {Promise} that resolves when the widget is available + * @returns {Promise} that resolves when the widget is the the + * requested state according to the `add` param */ static waitForRoomWidget(widgetId, roomId, add) { return new Promise((resolve, reject) => { From 3e6b3215cfe15532e19fd48288caf9fe23fd465e Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 14 Jun 2018 13:49:23 +0100 Subject: [PATCH 4/4] Update WidgetUtils.js Fix fixed comment --- src/WidgetUtils.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/WidgetUtils.js b/src/WidgetUtils.js index 98ee935f35..2e2dcf30cd 100644 --- a/src/WidgetUtils.js +++ b/src/WidgetUtils.js @@ -100,7 +100,7 @@ export default class WidgetUtils { * @param {string} widgetId The ID of the widget to wait for * @param {boolean} add True to wait for the widget to be added, * false to wait for it to be deleted. - * @returns {Promise} that resolves when the widget is the the + * @returns {Promise} that resolves when the widget is in the * requested state according to the `add` param */ static waitForUserWidget(widgetId, add) { @@ -147,7 +147,7 @@ export default class WidgetUtils { * @param {string} roomId The ID of the room to wait for the widget in * @param {boolean} add True to wait for the widget to be added, * false to wait for it to be deleted. - * @returns {Promise} that resolves when the widget is the the + * @returns {Promise} that resolves when the widget is in the * requested state according to the `add` param */ static waitForRoomWidget(widgetId, roomId, add) {