Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Distinguish room state and timeline events when dealing with widgets #28681

Merged
merged 9 commits into from
Jan 22, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 43 additions & 14 deletions src/stores/widgets/StopGapWidget.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,14 @@
* Please see LICENSE files in the repository root for full details.
*/

import { Room, MatrixEvent, MatrixEventEvent, MatrixClient, ClientEvent } from "matrix-js-sdk/src/matrix";
import {
Room,
MatrixEvent,
MatrixEventEvent,
MatrixClient,
ClientEvent,
RoomStateEvent,
} from "matrix-js-sdk/src/matrix";
import { KnownMembership } from "matrix-js-sdk/src/types";
import {
ClientWidgetApi,
Expand All @@ -26,7 +33,6 @@
WidgetApiFromWidgetAction,
WidgetKind,
} from "matrix-widget-api";
import { Optional } from "matrix-events-sdk";
import { EventEmitter } from "events";
import { logger } from "matrix-js-sdk/src/logger";

Expand Down Expand Up @@ -56,6 +62,7 @@
import Modal from "../../Modal";
import ErrorDialog from "../../components/views/dialogs/ErrorDialog";
import { SdkContextClass } from "../../contexts/SDKContext";
import { UPDATE_EVENT } from "../AsyncStore";

// TODO: Destroy all of this code

Expand Down Expand Up @@ -151,6 +158,7 @@
private mockWidget: ElementWidget;
private scalarToken?: string;
private roomId?: string;
private viewedRoomId: string | null = null;
robintown marked this conversation as resolved.
Show resolved Hide resolved
private kind: WidgetKind;
private readonly virtual: boolean;
private readUpToMap: { [roomId: string]: string } = {}; // room ID to event ID
Expand All @@ -177,17 +185,6 @@
this.stickyPromise = appTileProps.stickyPromise;
}

private get eventListenerRoomId(): Optional<string> {
// When widgets are listening to events, we need to make sure they're only
// receiving events for the right room. In particular, room widgets get locked
// to the room they were added in while account widgets listen to the currently
// active room.

if (this.roomId) return this.roomId;

return SdkContextClass.instance.roomViewStore.getRoomId();
}

public get widgetApi(): ClientWidgetApi | null {
return this.messaging;
}
Expand Down Expand Up @@ -259,6 +256,15 @@
});
}
};

private onRoomViewStoreUpdate = (): void => {
const roomId = SdkContextClass.instance.roomViewStore.getRoomId() ?? null;
if (roomId !== this.viewedRoomId) {
this.messaging!.setViewedRoomId(roomId);

Check failure on line 263 in src/stores/widgets/StopGapWidget.ts

View workflow job for this annotation

GitHub Actions / Typescript Syntax Check

Property 'setViewedRoomId' does not exist on type 'ClientWidgetApi'.
this.viewedRoomId = roomId;
}
};
robintown marked this conversation as resolved.
Show resolved Hide resolved

/**
* This starts the messaging for the widget if it is not in the state `started` yet.
* @param iframe the iframe the widget should use
Expand All @@ -285,6 +291,17 @@
this.messaging.on("capabilitiesNotified", () => this.emit("capabilitiesNotified"));
this.messaging.on(`action:${WidgetApiFromWidgetAction.OpenModalWidget}`, this.onOpenModal);

// When widgets are listening to events, we need to make sure they're only
// receiving events for the right room
if (this.roomId === undefined) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just read through and worked out what's going on here: seems a bit excessive to send the entire room state through to an account widget whenever you switch rooms? I don't see any capability negotiation going on here so seems like this will happen whether the widget actually wants it or not. I guess it is what the spec technically implies but is anything actually going to use it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's correct that matrix-widget-api is set up to re-sync the room state whenever an account widget encounters a new room. However, there is capability negotiation, and it will only read+push those state entries that match the widget's capabilities: https://github.com/matrix-org/matrix-widget-api/blob/61c45f78e09cafd3094c4a73892bd34be803bd98/src/ClientWidgetApi.ts#L1104

Is that already more along the lines of what you were expecting?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, I missed that it was done at the event type level, so that makes more sense.

// Account widgets listen to the currently active room
this.messaging.setViewedRoomId(SdkContextClass.instance.roomViewStore.getRoomId() ?? null);

Check failure on line 298 in src/stores/widgets/StopGapWidget.ts

View workflow job for this annotation

GitHub Actions / Typescript Syntax Check

Property 'setViewedRoomId' does not exist on type 'ClientWidgetApi'.
SdkContextClass.instance.roomViewStore.on(UPDATE_EVENT, this.onRoomViewStoreUpdate);
} else {
// Room widgets get looked to the room they were added in
robintown marked this conversation as resolved.
Show resolved Hide resolved
this.messaging.setViewedRoomId(this.roomId);

Check failure on line 302 in src/stores/widgets/StopGapWidget.ts

View workflow job for this annotation

GitHub Actions / Typescript Syntax Check

Property 'setViewedRoomId' does not exist on type 'ClientWidgetApi'.

Check failure on line 302 in src/stores/widgets/StopGapWidget.ts

View workflow job for this annotation

GitHub Actions / Jest (2)

StopGapWidget › should replace parameters in widget url template

TypeError: this.messaging.setViewedRoomId is not a function at StopGapWidget.setViewedRoomId [as startMessaging] (src/stores/widgets/StopGapWidget.ts:302:28) at Object.startMessaging (test/unit-tests/stores/widgets/StopGapWidget-test.ts:54:16)

Check failure on line 302 in src/stores/widgets/StopGapWidget.ts

View workflow job for this annotation

GitHub Actions / Jest (2)

StopGapWidget › feeds incoming to-device messages to the widget

TypeError: this.messaging.setViewedRoomId is not a function at StopGapWidget.setViewedRoomId [as startMessaging] (src/stores/widgets/StopGapWidget.ts:302:28) at Object.startMessaging (test/unit-tests/stores/widgets/StopGapWidget-test.ts:54:16)

Check failure on line 302 in src/stores/widgets/StopGapWidget.ts

View workflow job for this annotation

GitHub Actions / Jest (2)

StopGapWidget › feed event › feeds incoming event to the widget

TypeError: this.messaging.setViewedRoomId is not a function at StopGapWidget.setViewedRoomId [as startMessaging] (src/stores/widgets/StopGapWidget.ts:302:28) at Object.startMessaging (test/unit-tests/stores/widgets/StopGapWidget-test.ts:54:16)

Check failure on line 302 in src/stores/widgets/StopGapWidget.ts

View workflow job for this annotation

GitHub Actions / Jest (2)

StopGapWidget › feed event › should not feed incoming event to the widget if seen already

TypeError: this.messaging.setViewedRoomId is not a function at StopGapWidget.setViewedRoomId [as startMessaging] (src/stores/widgets/StopGapWidget.ts:302:28) at Object.startMessaging (test/unit-tests/stores/widgets/StopGapWidget-test.ts:54:16)

Check failure on line 302 in src/stores/widgets/StopGapWidget.ts

View workflow job for this annotation

GitHub Actions / Jest (2)

StopGapWidget › feed event › feeds decrypted events asynchronously

TypeError: this.messaging.setViewedRoomId is not a function at StopGapWidget.setViewedRoomId [as startMessaging] (src/stores/widgets/StopGapWidget.ts:302:28) at Object.startMessaging (test/unit-tests/stores/widgets/StopGapWidget-test.ts:54:16)

Check failure on line 302 in src/stores/widgets/StopGapWidget.ts

View workflow job for this annotation

GitHub Actions / Jest (2)

StopGapWidget › feed event › should not feed incoming event if not in timeline

TypeError: this.messaging.setViewedRoomId is not a function at StopGapWidget.setViewedRoomId [as startMessaging] (src/stores/widgets/StopGapWidget.ts:302:28) at Object.startMessaging (test/unit-tests/stores/widgets/StopGapWidget-test.ts:54:16)

Check failure on line 302 in src/stores/widgets/StopGapWidget.ts

View workflow job for this annotation

GitHub Actions / Jest (2)

StopGapWidget › feed event › feeds incoming event that is not in timeline but relates to unknown parent to the widget

TypeError: this.messaging.setViewedRoomId is not a function at StopGapWidget.setViewedRoomId [as startMessaging] (src/stores/widgets/StopGapWidget.ts:302:28) at Object.startMessaging (test/unit-tests/stores/widgets/StopGapWidget-test.ts:54:16)

Check failure on line 302 in src/stores/widgets/StopGapWidget.ts

View workflow job for this annotation

GitHub Actions / Jest (2)

StopGapWidget with stickyPromise › should wait for the sticky promise to resolve before starting messaging

TypeError: this.messaging.setViewedRoomId is not a function at StopGapWidget.setViewedRoomId [as startMessaging] (src/stores/widgets/StopGapWidget.ts:302:28) at Object.startMessaging (test/unit-tests/stores/widgets/StopGapWidget-test.ts:267:16)
}

// Always attach a handler for ViewRoom, but permission check it internally
this.messaging.on(`action:${ElementWidgetActions.ViewRoom}`, (ev: CustomEvent<IViewRoomApiRequest>) => {
ev.preventDefault(); // stop the widget API from auto-rejecting this
Expand Down Expand Up @@ -329,6 +346,7 @@
// Attach listeners for feeding events - the underlying widget classes handle permissions for us
this.client.on(ClientEvent.Event, this.onEvent);
this.client.on(MatrixEventEvent.Decrypted, this.onEventDecrypted);
this.client.on(RoomStateEvent.Events, this.onStateUpdate);
this.client.on(ClientEvent.ToDeviceEvent, this.onToDeviceEvent);

this.messaging.on(
Expand Down Expand Up @@ -457,8 +475,11 @@
WidgetMessagingStore.instance.stopMessaging(this.mockWidget, this.roomId);
this.messaging = null;

SdkContextClass.instance.roomViewStore.off(UPDATE_EVENT, this.onRoomViewStoreUpdate);

this.client.off(ClientEvent.Event, this.onEvent);
this.client.off(MatrixEventEvent.Decrypted, this.onEventDecrypted);
this.client.off(RoomStateEvent.Events, this.onStateUpdate);
this.client.off(ClientEvent.ToDeviceEvent, this.onToDeviceEvent);
}

Expand All @@ -471,6 +492,14 @@
this.feedEvent(ev);
};

private onStateUpdate = (ev: MatrixEvent): void => {
if (this.messaging === null) return;
const raw = ev.getEffectiveEvent();
this.messaging.feedStateUpdate(raw as IRoomEvent).catch((e) => {

Check failure on line 498 in src/stores/widgets/StopGapWidget.ts

View workflow job for this annotation

GitHub Actions / Typescript Syntax Check

Property 'feedStateUpdate' does not exist on type 'ClientWidgetApi'.

Check failure on line 498 in src/stores/widgets/StopGapWidget.ts

View workflow job for this annotation

GitHub Actions / Typescript Syntax Check

Parameter 'e' implicitly has an 'any' type.
robintown marked this conversation as resolved.
Show resolved Hide resolved
logger.error("Error sending state update to widget: ", e);
});
};

private onToDeviceEvent = async (ev: MatrixEvent): Promise<void> => {
await this.client.decryptEventIfNeeded(ev);
if (ev.isDecryptionFailure()) return;
Expand Down Expand Up @@ -570,7 +599,7 @@
this.eventsToFeed.add(ev);
} else {
const raw = ev.getEffectiveEvent();
this.messaging.feedEvent(raw as IRoomEvent, this.eventListenerRoomId!).catch((e) => {
this.messaging.feedEvent(raw as IRoomEvent).catch((e) => {

Check failure on line 602 in src/stores/widgets/StopGapWidget.ts

View workflow job for this annotation

GitHub Actions / Typescript Syntax Check

Expected 2 arguments, but got 1.
logger.error("Error sending event to widget: ", e);
});
}
Expand Down
96 changes: 37 additions & 59 deletions src/stores/widgets/StopGapWidgetDriver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import {
MatrixCapabilities,
OpenIDRequestState,
SimpleObservable,
Symbols,
Widget,
WidgetDriver,
WidgetEventCapability,
Expand All @@ -36,7 +35,6 @@ import {
IContent,
MatrixError,
MatrixEvent,
Room,
Direction,
THREAD_RELATION_TYPE,
SendDelayedEventResponse,
Expand Down Expand Up @@ -469,70 +467,44 @@ export class StopGapWidgetDriver extends WidgetDriver {
}
}

private pickRooms(roomIds?: (string | Symbols.AnyRoom)[]): Room[] {
const client = MatrixClientPeg.get();
if (!client) throw new Error("Not attached to a client");

const targetRooms = roomIds
? roomIds.includes(Symbols.AnyRoom)
? client.getVisibleRooms(SettingsStore.getValue("feature_dynamic_room_predecessors"))
: roomIds.map((r) => client.getRoom(r))
: [client.getRoom(SdkContextClass.instance.roomViewStore.getRoomId()!)];
return targetRooms.filter((r) => !!r) as Room[];
}

public async readRoomEvents(
public async readRoomTimeline(
roomId: string,
eventType: string,
msgtype: string | undefined,
limitPerRoom: number,
roomIds?: (string | Symbols.AnyRoom)[],
stateKey: string | undefined,
limit: number,
since: string | undefined,
): Promise<IRoomEvent[]> {
limitPerRoom = limitPerRoom > 0 ? Math.min(limitPerRoom, Number.MAX_SAFE_INTEGER) : Number.MAX_SAFE_INTEGER; // relatively arbitrary

const rooms = this.pickRooms(roomIds);
const allResults: IRoomEvent[] = [];
for (const room of rooms) {
const results: MatrixEvent[] = [];
const events = room.getLiveTimeline().getEvents(); // timelines are most recent last
for (let i = events.length - 1; i > 0; i--) {
if (results.length >= limitPerRoom) break;

const ev = events[i];
if (ev.getType() !== eventType || ev.isState()) continue;
if (eventType === EventType.RoomMessage && msgtype && msgtype !== ev.getContent()["msgtype"]) continue;
results.push(ev);
}

results.forEach((e) => allResults.push(e.getEffectiveEvent() as IRoomEvent));
limit = limit > 0 ? Math.min(limit, Number.MAX_SAFE_INTEGER) : Number.MAX_SAFE_INTEGER; // relatively arbitrary

const room = MatrixClientPeg.safeGet().getRoom(roomId);
if (room === null) return [];
const results: MatrixEvent[] = [];
const events = room.getLiveTimeline().getEvents(); // timelines are most recent last
for (let i = events.length - 1; i > 0; i--) {
const ev = events[i];
if (results.length >= limit) break;
if (since !== undefined && ev.getId() === since) break;

if (ev.getType() !== eventType || ev.isState()) continue;
if (eventType === EventType.RoomMessage && msgtype && msgtype !== ev.getContent()["msgtype"]) continue;
if (ev.getStateKey() !== undefined && stateKey !== undefined && ev.getStateKey() !== stateKey) continue;
results.push(ev);
}
return allResults;

return results.map((e) => e.getEffectiveEvent() as IRoomEvent);
}

public async readStateEvents(
eventType: string,
stateKey: string | undefined,
limitPerRoom: number,
roomIds?: (string | Symbols.AnyRoom)[],
): Promise<IRoomEvent[]> {
limitPerRoom = limitPerRoom > 0 ? Math.min(limitPerRoom, Number.MAX_SAFE_INTEGER) : Number.MAX_SAFE_INTEGER; // relatively arbitrary

const rooms = this.pickRooms(roomIds);
const allResults: IRoomEvent[] = [];
for (const room of rooms) {
const results: MatrixEvent[] = [];
const state = room.currentState.events.get(eventType);
if (state) {
if (stateKey === "" || !!stateKey) {
const forKey = state.get(stateKey);
if (forKey) results.push(forKey);
} else {
results.push(...Array.from(state.values()));
}
}
public async readRoomState(roomId: string, eventType: string, stateKey: string | undefined): Promise<IRoomEvent[]> {
robintown marked this conversation as resolved.
Show resolved Hide resolved
const room = MatrixClientPeg.safeGet().getRoom(roomId);
if (room === null) return [];
const state = room.getLiveTimeline().getState(Direction.Forward);
if (state === undefined) return [];

results.slice(0, limitPerRoom).forEach((e) => allResults.push(e.getEffectiveEvent() as IRoomEvent));
}
return allResults;
if (stateKey === undefined)
return state.getStateEvents(eventType).map((e) => e.getEffectiveEvent() as IRoomEvent);
const event = state.getStateEvents(eventType, stateKey);
return event === null ? [] : [event.getEffectiveEvent() as IRoomEvent];
}

public async askOpenID(observer: SimpleObservable<IOpenIDUpdate>): Promise<void> {
Expand Down Expand Up @@ -693,6 +665,12 @@ export class StopGapWidgetDriver extends WidgetDriver {
return { file: blob };
}

public getKnownRooms(): string[] {
robintown marked this conversation as resolved.
Show resolved Hide resolved
return MatrixClientPeg.safeGet()
.getVisibleRooms(SettingsStore.getValue("feature_dynamic_room_predecessors"))
.map((r) => r.roomId);
}

/**
* Expresses a {@link MatrixError} as a JSON payload
* for use by Widget API error responses.
Expand Down
Loading