From ee78ba3d613f997593b28b9bf4b3406818e418c4 Mon Sep 17 00:00:00 2001 From: Timo Date: Fri, 24 Jan 2025 14:54:46 +0100 Subject: [PATCH 1/8] fixes for reading room state --- src/ClientWidgetApi.ts | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/src/ClientWidgetApi.ts b/src/ClientWidgetApi.ts index 9c949b4..8ff01b5 100644 --- a/src/ClientWidgetApi.ts +++ b/src/ClientWidgetApi.ts @@ -530,18 +530,24 @@ export class ClientWidgetApi extends EventEmitter { // For backwards compatibility we still call the deprecated // readRoomEvents and readStateEvents methods in case the client isn't // letting us know the currently viewed room via setViewedRoomId - const events = - request.data.room_ids === undefined && askRoomIds.length === 0 - ? await (request.data.state_key === undefined - ? this.driver.readRoomEvents(request.data.type, msgtype, limit, null, since) - : this.driver.readStateEvents(request.data.type, stateKey, limit, null)) - : ( - await Promise.all( - askRoomIds.map((roomId) => - this.driver.readRoomTimeline(roomId, request.data.type, msgtype, stateKey, limit, since), - ), - ) - ).flat(1); + const events = request.data.room_ids === undefined && askRoomIds.length === 0 + ? await ( + // This returns [] with the current driver of Element Web. + // Add default implementations of the `readRoomEvents` and `readStateEvents` + // methods to use `readRoomTimeline` and `readRoomState` if they are not overwritten. + request.data.state_key === undefined + ? this.driver.readRoomEvents(request.data.type, msgtype, limit, null, since) + : this.driver.readStateEvents(request.data.type, stateKey, limit, null) + ) + : ( + request.data.state_key === undefined + ? await Promise.all(askRoomIds.map(roomId => + this.driver.readRoomTimeline(roomId, request.data.type, msgtype, stateKey, limit, since), + )) + : await Promise.all(askRoomIds.map(roomId => + this.driver.readRoomState(roomId, request.data.type, stateKey), + )) + ).flat(1); this.transport.reply(request, { events }); } From 71b14fc621cc439feef2a7b98671d6c54c7c9339 Mon Sep 17 00:00:00 2001 From: Timo Date: Fri, 24 Jan 2025 18:12:07 +0100 Subject: [PATCH 2/8] fix tests to mock readRoomState instead of readRoomTimeline --- src/ClientWidgetApi.ts | 43 +++++++++++++++++++++--------------- test/ClientWidgetApi-test.ts | 26 +++++----------------- 2 files changed, 31 insertions(+), 38 deletions(-) diff --git a/src/ClientWidgetApi.ts b/src/ClientWidgetApi.ts index 8ff01b5..a71a12c 100644 --- a/src/ClientWidgetApi.ts +++ b/src/ClientWidgetApi.ts @@ -530,24 +530,31 @@ export class ClientWidgetApi extends EventEmitter { // For backwards compatibility we still call the deprecated // readRoomEvents and readStateEvents methods in case the client isn't // letting us know the currently viewed room via setViewedRoomId - const events = request.data.room_ids === undefined && askRoomIds.length === 0 - ? await ( - // This returns [] with the current driver of Element Web. - // Add default implementations of the `readRoomEvents` and `readStateEvents` - // methods to use `readRoomTimeline` and `readRoomState` if they are not overwritten. - request.data.state_key === undefined - ? this.driver.readRoomEvents(request.data.type, msgtype, limit, null, since) - : this.driver.readStateEvents(request.data.type, stateKey, limit, null) - ) - : ( - request.data.state_key === undefined - ? await Promise.all(askRoomIds.map(roomId => - this.driver.readRoomTimeline(roomId, request.data.type, msgtype, stateKey, limit, since), - )) - : await Promise.all(askRoomIds.map(roomId => - this.driver.readRoomState(roomId, request.data.type, stateKey), - )) - ).flat(1); + const events = + request.data.room_ids === undefined && askRoomIds.length === 0 + ? await // This returns [] with the current driver of Element Web. + // Add default implementations of the `readRoomEvents` and `readStateEvents` + // methods to use `readRoomTimeline` and `readRoomState` if they are not overwritten. + (request.data.state_key === undefined + ? this.driver.readRoomEvents(request.data.type, msgtype, limit, null, since) + : this.driver.readStateEvents(request.data.type, stateKey, limit, null)) + : (request.data.state_key === undefined + ? await Promise.all( + askRoomIds.map((roomId) => + this.driver.readRoomTimeline( + roomId, + request.data.type, + msgtype, + stateKey, + limit, + since, + ), + ), + ) + : await Promise.all( + askRoomIds.map((roomId) => this.driver.readRoomState(roomId, request.data.type, stateKey)), + ) + ).flat(1); this.transport.reply(request, { events }); } diff --git a/test/ClientWidgetApi-test.ts b/test/ClientWidgetApi-test.ts index a9ae879..f3247f4 100644 --- a/test/ClientWidgetApi-test.ts +++ b/test/ClientWidgetApi-test.ts @@ -1534,7 +1534,7 @@ describe("ClientWidgetApi", () => { }); it("reads state events with any state key", async () => { - driver.readRoomTimeline.mockResolvedValue([ + driver.readRoomState.mockResolvedValue([ createRoomEvent({ type: "net.example.test", state_key: "A" }), createRoomEvent({ type: "net.example.test", state_key: "B" }), ]); @@ -1556,7 +1556,7 @@ describe("ClientWidgetApi", () => { emitEvent(new CustomEvent("", { detail: event })); await waitFor(() => { - expect(transport.reply).toBeCalledWith(event, { + expect(transport.reply).toHaveBeenCalledWith(event, { events: [ createRoomEvent({ type: "net.example.test", state_key: "A" }), createRoomEvent({ type: "net.example.test", state_key: "B" }), @@ -1564,14 +1564,7 @@ describe("ClientWidgetApi", () => { }); }); - expect(driver.readRoomTimeline).toBeCalledWith( - "!room-id", - "net.example.test", - undefined, - undefined, - 0, - undefined, - ); + expect(driver.readRoomState).toHaveBeenLastCalledWith("!room-id", "net.example.test", undefined); }); it("fails to read state events with any state key", async () => { @@ -1600,7 +1593,7 @@ describe("ClientWidgetApi", () => { }); it("reads state events with a specific state key", async () => { - driver.readRoomTimeline.mockResolvedValue([createRoomEvent({ type: "net.example.test", state_key: "B" })]); + driver.readRoomState.mockResolvedValue([createRoomEvent({ type: "net.example.test", state_key: "B" })]); const event: IReadEventFromWidgetActionRequest = { api: WidgetApiDirection.FromWidget, @@ -1619,19 +1612,12 @@ describe("ClientWidgetApi", () => { emitEvent(new CustomEvent("", { detail: event })); await waitFor(() => { - expect(transport.reply).toBeCalledWith(event, { + expect(transport.reply).toHaveBeenCalledWith(event, { events: [createRoomEvent({ type: "net.example.test", state_key: "B" })], }); }); - expect(driver.readRoomTimeline).toBeCalledWith( - "!room-id", - "net.example.test", - undefined, - "B", - 0, - undefined, - ); + expect(driver.readRoomState).toHaveBeenLastCalledWith("!room-id", "net.example.test", "B"); }); it("fails to read state events with a specific state key", async () => { From 9cf15e43c08d37bbd4700190c8e03697fcbb5e5e Mon Sep 17 00:00:00 2001 From: Timo Date: Fri, 24 Jan 2025 19:40:42 +0100 Subject: [PATCH 3/8] Check for MSC2762_UPDATE_STATE version to decide if read_event (state events) reads from the state or the timeline --- src/ClientWidgetApi.ts | 69 +++++++++++++++++++++--------------- test/ClientWidgetApi-test.ts | 49 +++++++++++++++++++++++++ 2 files changed, 90 insertions(+), 28 deletions(-) diff --git a/src/ClientWidgetApi.ts b/src/ClientWidgetApi.ts index a71a12c..dd5ea7c 100644 --- a/src/ClientWidgetApi.ts +++ b/src/ClientWidgetApi.ts @@ -354,6 +354,10 @@ export class ClientWidgetApi extends EventEmitter { }); } + private async supportsUpdateState(): Promise { + return (await this.getWidgetVersions()).includes(UnstableApiVersion.MSC2762_UPDATE_STATE); + } + private handleCapabilitiesRenegotiate(request: IRenegotiateCapabilitiesActionRequest): void { // acknowledge first this.transport.reply(request, {}); @@ -527,34 +531,43 @@ export class ClientWidgetApi extends EventEmitter { } } - // For backwards compatibility we still call the deprecated - // readRoomEvents and readStateEvents methods in case the client isn't - // letting us know the currently viewed room via setViewedRoomId - const events = - request.data.room_ids === undefined && askRoomIds.length === 0 - ? await // This returns [] with the current driver of Element Web. - // Add default implementations of the `readRoomEvents` and `readStateEvents` - // methods to use `readRoomTimeline` and `readRoomState` if they are not overwritten. - (request.data.state_key === undefined - ? this.driver.readRoomEvents(request.data.type, msgtype, limit, null, since) - : this.driver.readStateEvents(request.data.type, stateKey, limit, null)) - : (request.data.state_key === undefined - ? await Promise.all( - askRoomIds.map((roomId) => - this.driver.readRoomTimeline( - roomId, - request.data.type, - msgtype, - stateKey, - limit, - since, - ), - ), - ) - : await Promise.all( - askRoomIds.map((roomId) => this.driver.readRoomState(roomId, request.data.type, stateKey)), - ) - ).flat(1); + let events: IRoomEvent[]; + + if (request.data.room_ids === undefined && askRoomIds.length === 0) { + // For backwards compatibility we still call the deprecated + // readRoomEvents and readStateEvents methods in case the client isn't + // letting us know the currently viewed room via setViewedRoomId + events = await // This returns [] with the current driver of Element Web. + // Add default implementations of the `readRoomEvents` and `readStateEvents` + // methods to use `readRoomTimeline` and `readRoomState` if they are not overwritten. + (request.data.state_key === undefined + ? this.driver.readRoomEvents(request.data.type, msgtype, limit, null, since) + : this.driver.readStateEvents(request.data.type, stateKey, limit, null)); + } else if (await this.supportsUpdateState()) { + // Calling read_events with a stateKey still reads from the rooms timeline (not the room state). + events = ( + await Promise.all( + askRoomIds.map((roomId) => + this.driver.readRoomTimeline(roomId, request.data.type, msgtype, stateKey, limit, since), + ), + ) + ).flat(1); + } else { + // TODO: remove this once `UnstableApiVersion.MSC2762_UPDATE_STATE` becomes stable. + // Before version `MSC2762_UPDATE_STATE` we used readRoomState for read_events actions. + events = ( + request.data.state_key === undefined + ? await Promise.all( + askRoomIds.map((roomId) => + this.driver.readRoomTimeline(roomId, request.data.type, msgtype, stateKey, limit, since), + ), + ) + : await Promise.all( + askRoomIds.map((roomId) => this.driver.readRoomState(roomId, request.data.type, stateKey)), + ) + ).flat(1); + } + this.transport.reply(request, { events }); } diff --git a/test/ClientWidgetApi-test.ts b/test/ClientWidgetApi-test.ts index f3247f4..2c1e5f1 100644 --- a/test/ClientWidgetApi-test.ts +++ b/test/ClientWidgetApi-test.ts @@ -1437,6 +1437,7 @@ describe("ClientWidgetApi", () => { describe("org.matrix.msc2876.read_events action", () => { it("reads events from a specific room", async () => { const roomId = "!room:example.org"; + jest.spyOn(clientWidgetApi, "getWidgetVersions").mockReturnValue(new Promise((r) => r([]))); const event = createRoomEvent({ room_id: roomId, type: "net.example.test", content: "test" }); driver.readRoomTimeline.mockImplementation(async (rId) => { if (rId === roomId) return [event]; @@ -1481,6 +1482,7 @@ describe("ClientWidgetApi", () => { it("reads events from all rooms", async () => { const roomId = "!room:example.org"; const otherRoomId = "!other-room:example.org"; + jest.spyOn(clientWidgetApi, "getWidgetVersions").mockReturnValue(new Promise((r) => r([]))); const event = createRoomEvent({ room_id: roomId, type: "net.example.test", content: "test" }); const otherRoomEvent = createRoomEvent({ room_id: otherRoomId, type: "net.example.test", content: "hi" }); driver.getKnownRooms.mockReturnValue([roomId, otherRoomId]); @@ -1534,6 +1536,7 @@ describe("ClientWidgetApi", () => { }); it("reads state events with any state key", async () => { + jest.spyOn(clientWidgetApi, "getWidgetVersions").mockReturnValue(new Promise((r) => r([]))); driver.readRoomState.mockResolvedValue([ createRoomEvent({ type: "net.example.test", state_key: "A" }), createRoomEvent({ type: "net.example.test", state_key: "B" }), @@ -1593,6 +1596,7 @@ describe("ClientWidgetApi", () => { }); it("reads state events with a specific state key", async () => { + jest.spyOn(clientWidgetApi, "getWidgetVersions").mockReturnValue(new Promise((r) => r([]))); driver.readRoomState.mockResolvedValue([createRoomEvent({ type: "net.example.test", state_key: "B" })]); const event: IReadEventFromWidgetActionRequest = { @@ -1620,6 +1624,51 @@ describe("ClientWidgetApi", () => { expect(driver.readRoomState).toHaveBeenLastCalledWith("!room-id", "net.example.test", "B"); }); + it("reads state events with a specific state key from the timeline when using UnstableApiVersion.MSC2762_UPDATE_STATE", async () => { + jest.spyOn(clientWidgetApi, "getWidgetVersions").mockReturnValue(new Promise((r) => r(CurrentApiVersions))); + // with version MSC2762_UPDATE_STATE we wan the read Events action to read state events from the timeline. + driver.readRoomTimeline.mockResolvedValue([createRoomEvent({ type: "net.example.test", state_key: "B" })]); + + const event: IReadEventFromWidgetActionRequest = { + api: WidgetApiDirection.FromWidget, + widgetId: "test", + requestId: "0", + action: WidgetApiFromWidgetAction.MSC2876ReadEvents, + data: { + type: "net.example.test", + state_key: "B", + }, + }; + + await loadIframe(["org.matrix.msc2762.receive.state_event:net.example.test#B"]); + + clientWidgetApi.setViewedRoomId("!room-id"); + + // we clear the mock here because setViewedRoomId will push the room state and therefore read it + // from the driver. + driver.readRoomState.mockClear(); + // clearing this as well so it gets the same treatment as readRoomState for reference + driver.readRoomTimeline.mockClear(); + + emitEvent(new CustomEvent("", { detail: event })); + + await waitFor(() => { + expect(transport.reply).toHaveBeenCalledWith(event, { + events: [createRoomEvent({ type: "net.example.test", state_key: "B" })], + }); + }); + + expect(driver.readRoomTimeline).toHaveBeenLastCalledWith( + "!room-id", + "net.example.test", + undefined, + "B", + 0, + undefined, + ); + expect(driver.readRoomState).not.toHaveBeenCalled(); + }); + it("fails to read state events with a specific state key", async () => { const event: IReadEventFromWidgetActionRequest = { api: WidgetApiDirection.FromWidget, From 03c7300aadf7bcad819e9087366ffcd11e8927b6 Mon Sep 17 00:00:00 2001 From: Timo Date: Mon, 27 Jan 2025 15:10:36 +0100 Subject: [PATCH 4/8] add warning if we reach the deprecated code path. --- src/ClientWidgetApi.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/ClientWidgetApi.ts b/src/ClientWidgetApi.ts index dd5ea7c..1bd0e05 100644 --- a/src/ClientWidgetApi.ts +++ b/src/ClientWidgetApi.ts @@ -537,6 +537,9 @@ export class ClientWidgetApi extends EventEmitter { // For backwards compatibility we still call the deprecated // readRoomEvents and readStateEvents methods in case the client isn't // letting us know the currently viewed room via setViewedRoomId + console.warn( + "The widgetDriver uses deprecated behaviour:\n It does not set the viewedRoomId using `setViewedRoomId`", + ); events = await // This returns [] with the current driver of Element Web. // Add default implementations of the `readRoomEvents` and `readStateEvents` // methods to use `readRoomTimeline` and `readRoomState` if they are not overwritten. From de97f9e06745cb13c780688bd3537fa67aff580d Mon Sep 17 00:00:00 2001 From: Timo <16718859+toger5@users.noreply.github.com> Date: Mon, 27 Jan 2025 15:17:17 +0100 Subject: [PATCH 5/8] Review simpler return value Co-authored-by: Robin --- test/ClientWidgetApi-test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/ClientWidgetApi-test.ts b/test/ClientWidgetApi-test.ts index 2c1e5f1..0899341 100644 --- a/test/ClientWidgetApi-test.ts +++ b/test/ClientWidgetApi-test.ts @@ -1482,7 +1482,7 @@ describe("ClientWidgetApi", () => { it("reads events from all rooms", async () => { const roomId = "!room:example.org"; const otherRoomId = "!other-room:example.org"; - jest.spyOn(clientWidgetApi, "getWidgetVersions").mockReturnValue(new Promise((r) => r([]))); + jest.spyOn(clientWidgetApi, "getWidgetVersions").mockResolvedValue([]); const event = createRoomEvent({ room_id: roomId, type: "net.example.test", content: "test" }); const otherRoomEvent = createRoomEvent({ room_id: otherRoomId, type: "net.example.test", content: "hi" }); driver.getKnownRooms.mockReturnValue([roomId, otherRoomId]); From fa78686aac369137924bf79261961f3f10b7c321 Mon Sep 17 00:00:00 2001 From: Timo Date: Mon, 27 Jan 2025 15:29:18 +0100 Subject: [PATCH 6/8] more verbal comment --- src/ClientWidgetApi.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/ClientWidgetApi.ts b/src/ClientWidgetApi.ts index 1bd0e05..f9c9ced 100644 --- a/src/ClientWidgetApi.ts +++ b/src/ClientWidgetApi.ts @@ -537,6 +537,13 @@ export class ClientWidgetApi extends EventEmitter { // For backwards compatibility we still call the deprecated // readRoomEvents and readStateEvents methods in case the client isn't // letting us know the currently viewed room via setViewedRoomId + // + // This can be considered as a deprecated implementation. + // A driver should call `setViewedRoomId` on the widget messaging and implement the new readRoomState and readRoomTimeline + // Methods. + // This block makes sure that it is also possible to not use setViewedRoomId. + // readRoomTimeline and readRoomState are required however! Otherwise widget requests that include + // `room_ids` will fail. console.warn( "The widgetDriver uses deprecated behaviour:\n It does not set the viewedRoomId using `setViewedRoomId`", ); From edc8c333dbb606a5c6d227f5e9b63ad19f938e33 Mon Sep 17 00:00:00 2001 From: Timo Date: Mon, 27 Jan 2025 16:10:51 +0100 Subject: [PATCH 7/8] add default implementation for readRoomState. --- src/driver/WidgetDriver.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/driver/WidgetDriver.ts b/src/driver/WidgetDriver.ts index df92c03..9663e34 100644 --- a/src/driver/WidgetDriver.ts +++ b/src/driver/WidgetDriver.ts @@ -276,7 +276,7 @@ export abstract class WidgetDriver { * current values of the room state entries. */ public readRoomState(roomId: string, eventType: string, stateKey: string | undefined): Promise { - return Promise.resolve([]); + return this.readStateEvents(eventType, stateKey, Number.MAX_SAFE_INTEGER, [roomId]); } /** From a432c616ffa9f6a882a6ea013c91626b58cdf285 Mon Sep 17 00:00:00 2001 From: Timo Date: Mon, 27 Jan 2025 16:21:43 +0100 Subject: [PATCH 8/8] more review --- test/ClientWidgetApi-test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/ClientWidgetApi-test.ts b/test/ClientWidgetApi-test.ts index 0899341..2ac92ec 100644 --- a/test/ClientWidgetApi-test.ts +++ b/test/ClientWidgetApi-test.ts @@ -1437,7 +1437,7 @@ describe("ClientWidgetApi", () => { describe("org.matrix.msc2876.read_events action", () => { it("reads events from a specific room", async () => { const roomId = "!room:example.org"; - jest.spyOn(clientWidgetApi, "getWidgetVersions").mockReturnValue(new Promise((r) => r([]))); + jest.spyOn(clientWidgetApi, "getWidgetVersions").mockResolvedValue([]); const event = createRoomEvent({ room_id: roomId, type: "net.example.test", content: "test" }); driver.readRoomTimeline.mockImplementation(async (rId) => { if (rId === roomId) return [event]; @@ -1536,7 +1536,7 @@ describe("ClientWidgetApi", () => { }); it("reads state events with any state key", async () => { - jest.spyOn(clientWidgetApi, "getWidgetVersions").mockReturnValue(new Promise((r) => r([]))); + jest.spyOn(clientWidgetApi, "getWidgetVersions").mockResolvedValue([]); driver.readRoomState.mockResolvedValue([ createRoomEvent({ type: "net.example.test", state_key: "A" }), createRoomEvent({ type: "net.example.test", state_key: "B" }), @@ -1596,7 +1596,7 @@ describe("ClientWidgetApi", () => { }); it("reads state events with a specific state key", async () => { - jest.spyOn(clientWidgetApi, "getWidgetVersions").mockReturnValue(new Promise((r) => r([]))); + jest.spyOn(clientWidgetApi, "getWidgetVersions").mockResolvedValue([]); driver.readRoomState.mockResolvedValue([createRoomEvent({ type: "net.example.test", state_key: "B" })]); const event: IReadEventFromWidgetActionRequest = { @@ -1625,7 +1625,7 @@ describe("ClientWidgetApi", () => { }); it("reads state events with a specific state key from the timeline when using UnstableApiVersion.MSC2762_UPDATE_STATE", async () => { - jest.spyOn(clientWidgetApi, "getWidgetVersions").mockReturnValue(new Promise((r) => r(CurrentApiVersions))); + jest.spyOn(clientWidgetApi, "getWidgetVersions").mockResolvedValue(CurrentApiVersions); // with version MSC2762_UPDATE_STATE we wan the read Events action to read state events from the timeline. driver.readRoomTimeline.mockResolvedValue([createRoomEvent({ type: "net.example.test", state_key: "B" })]);