Fixes for reading room state (#126)

* fixes for reading room state

* fix tests to mock readRoomState instead of readRoomTimeline

* Check for MSC2762_UPDATE_STATE version to decide if read_event (state events) reads from the state or the timeline

* add warning if we reach the deprecated code path.

* Review simpler return value

Co-authored-by: Robin <[email protected]>

* more verbal comment

* add default implementation for readRoomState.

* more review

---------

Co-authored-by: Robin <[email protected]>

Author: toger5

src/ClientWidgetApi.ts

@@ -354,6 +354,10 @@ export class ClientWidgetApi extends EventEmitter {
         });
     }
 
+    private async supportsUpdateState(): Promise<boolean> {
+        return (await this.getWidgetVersions()).includes(UnstableApiVersion.MSC2762_UPDATE_STATE);
+    }
+
     private handleCapabilitiesRenegotiate(request: IRenegotiateCapabilitiesActionRequest): void {
         // acknowledge first
         this.transport.reply<IWidgetApiAcknowledgeResponseData>(request, {});
@@ -527,21 +531,53 @@ 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(
+        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
+            //
+            // 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`",
+            );
+            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),
                           ),
                       )
-                  ).flat(1);
+                    : await Promise.all(
+                          askRoomIds.map((roomId) => this.driver.readRoomState(roomId, request.data.type, stateKey)),
+                      )
+            ).flat(1);
+        }
+
         this.transport.reply<IReadEventFromWidgetResponseData>(request, { events });
     }
 

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<IRoomEvent[]> {
-        return Promise.resolve([]);
+        return this.readStateEvents(eventType, stateKey, Number.MAX_SAFE_INTEGER, [roomId]);
     }
 
     /**

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").mockResolvedValue([]);
             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").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]);
@@ -1534,7 +1536,8 @@ describe("ClientWidgetApi", () => {
         });
 
         it("reads state events with any state key", async () => {
-            driver.readRoomTimeline.mockResolvedValue([
+            jest.spyOn(clientWidgetApi, "getWidgetVersions").mockResolvedValue([]);
+            driver.readRoomState.mockResolvedValue([
                 createRoomEvent({ type: "net.example.test", state_key: "A" }),
                 createRoomEvent({ type: "net.example.test", state_key: "B" }),
             ]);
@@ -1556,22 +1559,15 @@ 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" }),
                     ],
                 });
             });
 
-            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,6 +1596,37 @@ describe("ClientWidgetApi", () => {
         });
 
         it("reads state events with a specific state key", async () => {
+            jest.spyOn(clientWidgetApi, "getWidgetVersions").mockResolvedValue([]);
+            driver.readRoomState.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");
+
+            emitEvent(new CustomEvent("", { detail: event }));
+
+            await waitFor(() => {
+                expect(transport.reply).toHaveBeenCalledWith(event, {
+                    events: [createRoomEvent({ type: "net.example.test", state_key: "B" })],
+                });
+            });
+
+            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").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" })]);
 
             const event: IReadEventFromWidgetActionRequest = {
@@ -1614,24 +1641,32 @@ describe("ClientWidgetApi", () => {
             };
 
             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).toBeCalledWith(event, {
+                expect(transport.reply).toHaveBeenCalledWith(event, {
                     events: [createRoomEvent({ type: "net.example.test", state_key: "B" })],
                 });
             });
 
-            expect(driver.readRoomTimeline).toBeCalledWith(
+            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 () => {