From 48d2715ac43fd941ec2c1e6c4fe54f4cfdf44bd5 Mon Sep 17 00:00:00 2001 From: Andy Balaam Date: Mon, 23 Jan 2023 17:09:44 +0000 Subject: [PATCH 1/2] Implement MSC3946 for getVisibleRooms --- spec/unit/matrix-client.spec.ts | 147 ++++++++++++++++++++++++++++++++ spec/unit/room.spec.ts | 46 ++++++++++ src/@types/event.ts | 1 + src/client.ts | 8 +- src/models/room.ts | 14 ++- 5 files changed, 213 insertions(+), 3 deletions(-) diff --git a/spec/unit/matrix-client.spec.ts b/spec/unit/matrix-client.spec.ts index 163a77b5b35..f656a82bc4a 100644 --- a/spec/unit/matrix-client.spec.ts +++ b/spec/unit/matrix-client.spec.ts @@ -2234,7 +2234,78 @@ describe("MatrixClient", function () { }); } + function predecessorEvent(newRoomId: string, predecessorRoomId: string): MatrixEvent { + return new MatrixEvent({ + content: { + predecessor_room_id: predecessorRoomId, + }, + event_id: `predecessor_event_id_pred_${predecessorRoomId}`, + origin_server_ts: 1432735824653, + room_id: newRoomId, + sender: "@daryl:alexandria.example.com", + state_key: "", + type: "org.matrix.msc3946.room_predecessor", + }); + } + describe("getVisibleRooms", () => { + function setUpReplacedRooms(): { + room1: Room; + room2: Room; + replacedByCreate1: Room; + replacedByCreate2: Room; + replacedByDynamicPredecessor1: Room; + replacedByDynamicPredecessor2: Room; + } { + const room1 = new Room("room1", client, "@carol:alexandria.example.com"); + const replacedByCreate1 = new Room("replacedByCreate1", client, "@carol:alexandria.example.com"); + const replacedByCreate2 = new Room("replacedByCreate2", client, "@carol:alexandria.example.com"); + const replacedByDynamicPredecessor1 = new Room("dyn1", client, "@carol:alexandria.example.com"); + const replacedByDynamicPredecessor2 = new Room("dyn2", client, "@carol:alexandria.example.com"); + const room2 = new Room("room2", client, "@daryl:alexandria.example.com"); + client.store = new StubStore(); + client.store.getRooms = () => [ + room1, + replacedByCreate1, + replacedByCreate2, + replacedByDynamicPredecessor1, + replacedByDynamicPredecessor2, + room2, + ]; + room1.addLiveEvents( + [ + roomCreateEvent(room1.roomId, replacedByCreate1.roomId), + predecessorEvent(room1.roomId, replacedByDynamicPredecessor1.roomId), + ], + {}, + ); + room2.addLiveEvents( + [ + roomCreateEvent(room2.roomId, replacedByCreate2.roomId), + predecessorEvent(room2.roomId, replacedByDynamicPredecessor2.roomId), + ], + {}, + ); + replacedByCreate1.addLiveEvents([tombstoneEvent(room1.roomId, replacedByCreate1.roomId)], {}); + replacedByCreate2.addLiveEvents([tombstoneEvent(room2.roomId, replacedByCreate2.roomId)], {}); + replacedByDynamicPredecessor1.addLiveEvents( + [tombstoneEvent(room1.roomId, replacedByDynamicPredecessor1.roomId)], + {}, + ); + replacedByDynamicPredecessor2.addLiveEvents( + [tombstoneEvent(room2.roomId, replacedByDynamicPredecessor2.roomId)], + {}, + ); + + return { + room1, + room2, + replacedByCreate1, + replacedByCreate2, + replacedByDynamicPredecessor1, + replacedByDynamicPredecessor2, + }; + } it("Returns an empty list if there are no rooms", () => { client.store = new StubStore(); client.store.getRooms = () => []; @@ -2275,6 +2346,82 @@ describe("MatrixClient", function () { expect(rooms).toContain(room1); expect(rooms).toContain(room2); }); + + it("Ignores m.predecessor if we don't ask to use it", () => { + // Given 6 rooms, 2 of which have been replaced, and 2 of which WERE + // replaced by create events, but are now NOT replaced, because an + // m.predecessor event has changed the room's predecessor. + const { + room1, + room2, + replacedByCreate1, + replacedByCreate2, + replacedByDynamicPredecessor1, + replacedByDynamicPredecessor2, + } = setUpReplacedRooms(); + + // When we ask for the visible rooms + const rooms = client.getVisibleRooms(); // Don't supply msc3946ProcessDynamicPredecessor + + // Then we only get the ones that have not been replaced + expect(rooms).not.toContain(replacedByCreate1); + expect(rooms).not.toContain(replacedByCreate2); + expect(rooms).toContain(replacedByDynamicPredecessor1); + expect(rooms).toContain(replacedByDynamicPredecessor2); + expect(rooms).toContain(room1); + expect(rooms).toContain(room2); + }); + + it("Considers rooms replaced with m.predecessor events to be replaced", () => { + // Given 6 rooms, 2 of which have been replaced, and 2 of which WERE + // replaced by create events, but are now NOT replaced, because an + // m.predecessor event has changed the room's predecessor. + const { + room1, + room2, + replacedByCreate1, + replacedByCreate2, + replacedByDynamicPredecessor1, + replacedByDynamicPredecessor2, + } = setUpReplacedRooms(); + + // When we ask for the visible rooms + const useMsc3946 = true; + const rooms = client.getVisibleRooms(useMsc3946); + + // Then we only get the ones that have not been replaced + expect(rooms).not.toContain(replacedByDynamicPredecessor1); + expect(rooms).not.toContain(replacedByDynamicPredecessor2); + expect(rooms).toContain(replacedByCreate1); + expect(rooms).toContain(replacedByCreate2); + expect(rooms).toContain(room1); + expect(rooms).toContain(room2); + }); + + it("Ignores m.predecessor if we don't ask to use it", () => { + // Given 6 rooms, 2 of which have been replaced, and 2 of which WERE + // replaced by create events, but are now NOT replaced, because an + // m.predecessor event has changed the room's predecessor. + const { + room1, + room2, + replacedByCreate1, + replacedByCreate2, + replacedByDynamicPredecessor1, + replacedByDynamicPredecessor2, + } = setUpReplacedRooms(); + + // When we ask for the visible rooms + const rooms = client.getVisibleRooms(); // Don't supply msc3946ProcessDynamicPredecessor + + // Then we only get the ones that have not been replaced + expect(rooms).not.toContain(replacedByCreate1); + expect(rooms).not.toContain(replacedByCreate2); + expect(rooms).toContain(replacedByDynamicPredecessor1); + expect(rooms).toContain(replacedByDynamicPredecessor2); + expect(rooms).toContain(room1); + expect(rooms).toContain(room2); + }); }); describe("getRoomUpgradeHistory", () => { diff --git a/spec/unit/room.spec.ts b/spec/unit/room.spec.ts index 38fc2cdc42a..e90d1d756f5 100644 --- a/spec/unit/room.spec.ts +++ b/spec/unit/room.spec.ts @@ -3266,6 +3266,20 @@ describe("Room", function () { }); } + function predecessorEvent(newRoomId: string, predecessorRoomId: string): MatrixEvent { + return new MatrixEvent({ + content: { + predecessor_room_id: predecessorRoomId, + }, + event_id: `predecessor_event_id_pred_${predecessorRoomId}`, + origin_server_ts: 1432735824653, + room_id: newRoomId, + sender: "@daryl:alexandria.example.com", + state_key: "", + type: "org.matrix.msc3946.room_predecessor", + }); + } + it("Returns null if there is no create event", () => { const room = new Room("roomid", client!, "@u:example.com"); expect(room.findPredecessorRoomId()).toBeNull(); @@ -3282,5 +3296,37 @@ describe("Room", function () { room.addLiveEvents([roomCreateEvent("roomid", "replacedroomid")]); expect(room.findPredecessorRoomId()).toBe("replacedroomid"); }); + + it("Prefers the m.predecessor event if one exists", () => { + const room = new Room("roomid", client!, "@u:example.com"); + room.addLiveEvents([ + roomCreateEvent("roomid", "replacedroomid"), + predecessorEvent("roomid", "otherreplacedroomid"), + ]); + const useMsc3946 = true; + expect(room.findPredecessorRoomId(useMsc3946)).toBe("otherreplacedroomid"); + }); + + it("Ignores the m.predecessor event if we don't ask to use it", () => { + const room = new Room("roomid", client!, "@u:example.com"); + room.addLiveEvents([ + roomCreateEvent("roomid", "replacedroomid"), + predecessorEvent("roomid", "otherreplacedroomid"), + ]); + // Don't provide an argument for msc3946ProcessDynamicPredecessor - + // we should ignore the predecessor event. + expect(room.findPredecessorRoomId()).toBe("replacedroomid"); + }); + + it("Ignores the m.predecessor event and returns null if we don't ask to use it", () => { + const room = new Room("roomid", client!, "@u:example.com"); + room.addLiveEvents([ + roomCreateEvent("roomid", null), // Create event has no predecessor + predecessorEvent("roomid", "otherreplacedroomid"), + ]); + // Don't provide an argument for msc3946ProcessDynamicPredecessor - + // we should ignore the predecessor event. + expect(room.findPredecessorRoomId()).toBeNull(); + }); }); }); diff --git a/src/@types/event.ts b/src/@types/event.ts index 2859077218d..17af8df0272 100644 --- a/src/@types/event.ts +++ b/src/@types/event.ts @@ -33,6 +33,7 @@ export enum EventType { RoomGuestAccess = "m.room.guest_access", RoomServerAcl = "m.room.server_acl", RoomTombstone = "m.room.tombstone", + RoomPredecessor = "org.matrix.msc3946.room_predecessor", SpaceChild = "m.space.child", SpaceParent = "m.space.parent", diff --git a/src/client.ts b/src/client.ts index ae2afe9ca93..ca7b7d75e28 100644 --- a/src/client.ts +++ b/src/client.ts @@ -3780,14 +3780,18 @@ export class MatrixClient extends TypedEventEmitter { } /** + * @param msc3946ProcessDynamicPredecessor - if true, look for an + * m.room.predecessor state event and + * use it if found (MSC3946). * @returns the ID of the room that was this room's predecessor, or null if * this room has no predecessor. */ - public findPredecessorRoomId(): string | null { + public findPredecessorRoomId(msc3946ProcessDynamicPredecessor = false): string | null { const currentState = this.getLiveTimeline().getState(EventTimeline.FORWARDS); if (!currentState) { return null; } + if (msc3946ProcessDynamicPredecessor) { + const predecessorEvent = currentState.getStateEvents(EventType.RoomPredecessor, ""); + if (predecessorEvent) { + const roomId = predecessorEvent.getContent()["predecessor_room_id"]; + if (roomId) { + return roomId; + } + } + } const createEvent = currentState.getStateEvents(EventType.RoomCreate, ""); if (createEvent) { From cb7f327b3009b2ca01504d5e6b9f7e2da37f954a Mon Sep 17 00:00:00 2001 From: Andy Balaam Date: Tue, 24 Jan 2023 13:43:20 +0000 Subject: [PATCH 2/2] Implement MSC3946 for getRoomUpgradeHistory --- spec/unit/matrix-client.spec.ts | 70 +++++++++++++++++++++++++++++++++ src/client.ts | 25 +++++++----- 2 files changed, 86 insertions(+), 9 deletions(-) diff --git a/spec/unit/matrix-client.spec.ts b/spec/unit/matrix-client.spec.ts index f656a82bc4a..24d3fed7727 100644 --- a/spec/unit/matrix-client.spec.ts +++ b/spec/unit/matrix-client.spec.ts @@ -2458,6 +2458,52 @@ describe("MatrixClient", function () { return [room1, room2, room3, room4]; } + /** + * Creates 2 alternate chains of room history: one using create + * events, and one using MSC2946 predecessor+tombstone events. + * + * Using create, history looks like: + * room1->room2->room3->room4 (but note we do not create tombstones) + * + * Using predecessor+tombstone, history looks like: + * dynRoom1->dynRoom2->room3->dynRoom4->dynRoom4 + * + * @returns [room1, room2, room3, room4, dynRoom1, dynRoom2, + * dynRoom4, dynRoom5]. + */ + function createDynamicRoomHistory(): [Room, Room, Room, Room, Room, Room, Room, Room] { + // Don't create tombstones for the old versions - we generally + // expect only one tombstone in a room, and we are confused by + // anything else. + const creates = true; + const tombstones = false; + const [room1, room2, room3, room4] = createRoomHistory(creates, tombstones); + const dynRoom1 = new Room("dynRoom1", client, "@rick:grimes.example.com"); + const dynRoom2 = new Room("dynRoom2", client, "@rick:grimes.example.com"); + const dynRoom4 = new Room("dynRoom4", client, "@rick:grimes.example.com"); + const dynRoom5 = new Room("dynRoom5", client, "@rick:grimes.example.com"); + + dynRoom1.addLiveEvents([tombstoneEvent(dynRoom2.roomId, dynRoom1.roomId)], {}); + dynRoom2.addLiveEvents([predecessorEvent(dynRoom2.roomId, dynRoom1.roomId)]); + + dynRoom2.addLiveEvents([tombstoneEvent(room3.roomId, dynRoom2.roomId)], {}); + room3.addLiveEvents([predecessorEvent(room3.roomId, dynRoom2.roomId)]); + + room3.addLiveEvents([tombstoneEvent(dynRoom4.roomId, room3.roomId)], {}); + dynRoom4.addLiveEvents([predecessorEvent(dynRoom4.roomId, room3.roomId)]); + + dynRoom4.addLiveEvents([tombstoneEvent(dynRoom5.roomId, dynRoom4.roomId)], {}); + dynRoom5.addLiveEvents([predecessorEvent(dynRoom5.roomId, dynRoom4.roomId)]); + + mocked(store.getRoom) + .mockClear() + .mockImplementation((roomId: string) => { + return { room1, room2, room3, room4, dynRoom1, dynRoom2, dynRoom4, dynRoom5 }[roomId] || null; + }); + + return [room1, room2, room3, room4, dynRoom1, dynRoom2, dynRoom4, dynRoom5]; + } + it("Returns an empty list if room does not exist", () => { const history = client.getRoomUpgradeHistory("roomthatdoesnotexist"); expect(history).toHaveLength(0); @@ -2600,6 +2646,30 @@ describe("MatrixClient", function () { room4.roomId, ]); }); + + it("Returns the predecessors and subsequent rooms using MSC3945 dynamic room predecessors", () => { + const [, , room3, , dynRoom1, dynRoom2, dynRoom4, dynRoom5] = createDynamicRoomHistory(); + const useMsc3946 = true; + const verifyLinks = false; + const history = client.getRoomUpgradeHistory(room3.roomId, verifyLinks, useMsc3946); + expect(history.map((room) => room.roomId)).toEqual([ + dynRoom1.roomId, + dynRoom2.roomId, + room3.roomId, + dynRoom4.roomId, + dynRoom5.roomId, + ]); + }); + + it("When not asking for MSC3946, verified history without tombstones is empty", () => { + // There no tombstones to match the create events + const [, , room3] = createDynamicRoomHistory(); + const useMsc3946 = false; + const verifyLinks = true; + const history = client.getRoomUpgradeHistory(room3.roomId, verifyLinks, useMsc3946); + // So we get no history back + expect(history.map((room) => room.roomId)).toEqual([room3.roomId]); + }); }); }); }); diff --git a/src/client.ts b/src/client.ts index ca7b7d75e28..ac0eb73748c 100644 --- a/src/client.ts +++ b/src/client.ts @@ -4990,24 +4990,31 @@ export class MatrixClient extends TypedEventEmitter