From fa800796c78978d74634c549c074ef70b3395e4f Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Wed, 6 Oct 2021 09:41:57 +0100 Subject: [PATCH 1/3] Respect tombstones in locally known rooms for Space children --- src/components/structures/SpaceHierarchy.tsx | 43 ++++++++++++++++---- src/stores/SpaceStore.ts | 31 ++++++++++++-- 2 files changed, 61 insertions(+), 13 deletions(-) diff --git a/src/components/structures/SpaceHierarchy.tsx b/src/components/structures/SpaceHierarchy.tsx index ed87b04c8a..8e1a472984 100644 --- a/src/components/structures/SpaceHierarchy.tsx +++ b/src/components/structures/SpaceHierarchy.tsx @@ -15,17 +15,17 @@ limitations under the License. */ import React, { + Dispatch, + KeyboardEvent, + KeyboardEventHandler, ReactNode, + SetStateAction, useCallback, + useContext, useEffect, useMemo, useRef, useState, - KeyboardEvent, - KeyboardEventHandler, - useContext, - SetStateAction, - Dispatch, } from "react"; import { Room } from "matrix-js-sdk/src/models/room"; import { RoomHierarchy } from "matrix-js-sdk/src/room-hierarchy"; @@ -33,7 +33,8 @@ import { EventType, RoomType } from "matrix-js-sdk/src/@types/event"; import { IHierarchyRelation, IHierarchyRoom } from "matrix-js-sdk/src/@types/spaces"; import { MatrixClient } from "matrix-js-sdk/src/matrix"; import classNames from "classnames"; -import { sortBy } from "lodash"; +import { sortBy, uniqBy } from "lodash"; +import { GuestAccess, HistoryVisibility } from "matrix-js-sdk/src/@types/partials"; import dis from "../../dispatcher/dispatcher"; import defaultDispatcher from "../../dispatcher/dispatcher"; @@ -48,7 +49,7 @@ import { mediaFromMxc } from "../../customisations/Media"; import InfoTooltip from "../views/elements/InfoTooltip"; import TextWithTooltip from "../views/elements/TextWithTooltip"; import { useStateToggle } from "../../hooks/useStateToggle"; -import { getChildOrder } from "../../stores/SpaceStore"; +import SpaceStore, { getChildOrder } from "../../stores/SpaceStore"; import AccessibleTooltipButton from "../views/elements/AccessibleTooltipButton"; import { linkifyElement } from "../../HtmlUtils"; import { useDispatcher } from "../../hooks/useDispatcher"; @@ -333,6 +334,29 @@ interface IHierarchyLevelProps { onToggleClick?(parentId: string, childId: string): void; } +const toLocalRoom = (cli: MatrixClient, room: IHierarchyRoom, upgradedRoomMap: Map): IHierarchyRoom => { + const cliRoom = cli.getRoom(SpaceStore.instance.findMostUpgradedVersion(room.room_id, upgradedRoomMap)); + if (cliRoom) { + return { + ...room, + room_id: cliRoom.roomId, + room_type: cliRoom.getType(), + name: cliRoom.name, + topic: cliRoom.currentState.getStateEvents(EventType.RoomTopic, "")?.getContent().topic, + avatar_url: cliRoom.getMxcAvatarUrl(), + canonical_alias: cliRoom.getCanonicalAlias(), + aliases: cliRoom.getAltAliases(), + world_readable: cliRoom.currentState.getStateEvents(EventType.RoomHistoryVisibility, "")?.getContent() + .history_visibility === HistoryVisibility.WorldReadable, + guest_can_join: cliRoom.currentState.getStateEvents(EventType.RoomGuestAccess, "")?.getContent() + .guest_access === GuestAccess.CanJoin, + num_joined_members: cliRoom.getJoinedMemberCount(), + }; + } + + return room; +}; + export const HierarchyLevel = ({ root, roomSet, @@ -350,10 +374,11 @@ export const HierarchyLevel = ({ return getChildOrder(ev.content.order, ev.origin_server_ts, ev.state_key); }); + const upgradedRoomMap = new Map(); const [subspaces, childRooms] = sortedChildren.reduce((result, ev: IHierarchyRelation) => { const room = hierarchy.roomMap.get(ev.state_key); if (room && roomSet.has(room)) { - result[room.room_type === RoomType.Space ? 0 : 1].push(room); + result[room.room_type === RoomType.Space ? 0 : 1].push(toLocalRoom(cli, room, upgradedRoomMap)); } return result; }, [[] as IHierarchyRoom[], [] as IHierarchyRoom[]]); @@ -361,7 +386,7 @@ export const HierarchyLevel = ({ const newParents = new Set(parents).add(root.room_id); return { - childRooms.map(room => ( + uniqBy(childRooms, "room_id").map(room => ( { const createTs = childRoom?.currentState.getStateEvents(EventType.RoomCreate, "")?.getTs(); return getChildOrder(ev.getContent().order, createTs, roomId); }).map(ev => { - return this.matrixClient.getRoom(ev.getStateKey()); + return this.matrixClient.getRoom(this.findMostUpgradedVersion(ev.getStateKey())); }).filter(room => { return room?.getMyMembership() === "join" || room?.getMyMembership() === "invite"; }) || []; @@ -452,6 +452,28 @@ export class SpaceStoreClass extends AsyncStoreWithClient { this.onRoomsUpdate(); }; + // Utility to walk tombstones and find the most updated variant of the given room, + // takes a Map to enable caching of the responses given the recursive nature of the function. + public findMostUpgradedVersion( + roomId: string, + upgradedRoomMap?: Map, + seen= new Set(), + ): string { + if (seen.has(roomId)) return roomId; + if (upgradedRoomMap?.has(roomId)) return upgradedRoomMap.get(roomId); + const room = this.matrixClient.getRoom(roomId); + const tombstone = room?.currentState.getStateEvents(EventType.RoomTombstone, ""); + const replacementRoom = tombstone?.getContent().replacement_room; + if (replacementRoom && this.matrixClient.getRoom(replacementRoom)?.getMyMembership() === "join") { + seen.add(roomId); + const result = this.findMostUpgradedVersion(replacementRoom, upgradedRoomMap); + upgradedRoomMap?.set(roomId, result); + return result; + } + upgradedRoomMap?.set(roomId, roomId); + return roomId; + } + private onRoomsUpdate = throttle(() => { // TODO resolve some updates as deltas const visibleRooms = this.matrixClient.getVisibleRooms(); @@ -479,6 +501,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { }); }); + const upgradedRoomMap = new Map(); this.rootSpaces.forEach(s => { // traverse each space tree in DFS to build up the supersets as you go up, // reusing results from like subtrees. @@ -491,7 +514,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { } const [childSpaces, childRooms] = partitionSpacesAndRooms(this.getChildren(spaceId)); - const roomIds = new Set(childRooms.map(r => r.roomId)); + const roomIds = new Set(childRooms.map(r => this.findMostUpgradedVersion(r.roomId, upgradedRoomMap))); const space = this.matrixClient?.getRoom(spaceId); // Add relevant DMs @@ -505,11 +528,11 @@ export class SpaceStoreClass extends AsyncStoreWithClient { const newPath = new Set(parentPath).add(spaceId); childSpaces.forEach(childSpace => { fn(childSpace.roomId, newPath)?.forEach(roomId => { - roomIds.add(roomId); + roomIds.add(this.findMostUpgradedVersion(roomId, upgradedRoomMap)); }); }); hiddenChildren.get(spaceId)?.forEach(roomId => { - roomIds.add(roomId); + roomIds.add(this.findMostUpgradedVersion(roomId, upgradedRoomMap)); }); this.spaceFilteredRooms.set(spaceId, roomIds); return roomIds; From e470d7d030562d032f568296e29f5aaec6755950 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 7 Oct 2021 13:19:51 +0100 Subject: [PATCH 2/3] Make use of MatrixClient::getRoomUpgradeHistory --- src/components/structures/SpaceHierarchy.tsx | 10 ++--- src/stores/SpaceStore.ts | 41 ++++++-------------- 2 files changed, 17 insertions(+), 34 deletions(-) diff --git a/src/components/structures/SpaceHierarchy.tsx b/src/components/structures/SpaceHierarchy.tsx index 8e1a472984..38d4129294 100644 --- a/src/components/structures/SpaceHierarchy.tsx +++ b/src/components/structures/SpaceHierarchy.tsx @@ -49,7 +49,7 @@ import { mediaFromMxc } from "../../customisations/Media"; import InfoTooltip from "../views/elements/InfoTooltip"; import TextWithTooltip from "../views/elements/TextWithTooltip"; import { useStateToggle } from "../../hooks/useStateToggle"; -import SpaceStore, { getChildOrder } from "../../stores/SpaceStore"; +import { getChildOrder } from "../../stores/SpaceStore"; import AccessibleTooltipButton from "../views/elements/AccessibleTooltipButton"; import { linkifyElement } from "../../HtmlUtils"; import { useDispatcher } from "../../hooks/useDispatcher"; @@ -334,8 +334,9 @@ interface IHierarchyLevelProps { onToggleClick?(parentId: string, childId: string): void; } -const toLocalRoom = (cli: MatrixClient, room: IHierarchyRoom, upgradedRoomMap: Map): IHierarchyRoom => { - const cliRoom = cli.getRoom(SpaceStore.instance.findMostUpgradedVersion(room.room_id, upgradedRoomMap)); +const toLocalRoom = (cli: MatrixClient, room: IHierarchyRoom): IHierarchyRoom => { + const history = cli.getRoomUpgradeHistory(room.room_id, true); + const cliRoom = history[history.length - 1]; if (cliRoom) { return { ...room, @@ -374,11 +375,10 @@ export const HierarchyLevel = ({ return getChildOrder(ev.content.order, ev.origin_server_ts, ev.state_key); }); - const upgradedRoomMap = new Map(); const [subspaces, childRooms] = sortedChildren.reduce((result, ev: IHierarchyRelation) => { const room = hierarchy.roomMap.get(ev.state_key); if (room && roomSet.has(room)) { - result[room.room_type === RoomType.Space ? 0 : 1].push(toLocalRoom(cli, room, upgradedRoomMap)); + result[room.room_type === RoomType.Space ? 0 : 1].push(toLocalRoom(cli, room)); } return result; }, [[] as IHierarchyRoom[], [] as IHierarchyRoom[]]); diff --git a/src/stores/SpaceStore.ts b/src/stores/SpaceStore.ts index e8bdce738b..3b28cda57f 100644 --- a/src/stores/SpaceStore.ts +++ b/src/stores/SpaceStore.ts @@ -283,7 +283,8 @@ export class SpaceStoreClass extends AsyncStoreWithClient { const createTs = childRoom?.currentState.getStateEvents(EventType.RoomCreate, "")?.getTs(); return getChildOrder(ev.getContent().order, createTs, roomId); }).map(ev => { - return this.matrixClient.getRoom(this.findMostUpgradedVersion(ev.getStateKey())); + const history = this.matrixClient.getRoomUpgradeHistory(ev.getStateKey(), true); + return history[history.length - 1]; }).filter(room => { return room?.getMyMembership() === "join" || room?.getMyMembership() === "invite"; }) || []; @@ -452,28 +453,6 @@ export class SpaceStoreClass extends AsyncStoreWithClient { this.onRoomsUpdate(); }; - // Utility to walk tombstones and find the most updated variant of the given room, - // takes a Map to enable caching of the responses given the recursive nature of the function. - public findMostUpgradedVersion( - roomId: string, - upgradedRoomMap?: Map, - seen= new Set(), - ): string { - if (seen.has(roomId)) return roomId; - if (upgradedRoomMap?.has(roomId)) return upgradedRoomMap.get(roomId); - const room = this.matrixClient.getRoom(roomId); - const tombstone = room?.currentState.getStateEvents(EventType.RoomTombstone, ""); - const replacementRoom = tombstone?.getContent().replacement_room; - if (replacementRoom && this.matrixClient.getRoom(replacementRoom)?.getMyMembership() === "join") { - seen.add(roomId); - const result = this.findMostUpgradedVersion(replacementRoom, upgradedRoomMap); - upgradedRoomMap?.set(roomId, result); - return result; - } - upgradedRoomMap?.set(roomId, roomId); - return roomId; - } - private onRoomsUpdate = throttle(() => { // TODO resolve some updates as deltas const visibleRooms = this.matrixClient.getVisibleRooms(); @@ -501,7 +480,6 @@ export class SpaceStoreClass extends AsyncStoreWithClient { }); }); - const upgradedRoomMap = new Map(); this.rootSpaces.forEach(s => { // traverse each space tree in DFS to build up the supersets as you go up, // reusing results from like subtrees. @@ -514,7 +492,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { } const [childSpaces, childRooms] = partitionSpacesAndRooms(this.getChildren(spaceId)); - const roomIds = new Set(childRooms.map(r => this.findMostUpgradedVersion(r.roomId, upgradedRoomMap))); + const roomIds = new Set(childRooms.map(r => r.roomId)); const space = this.matrixClient?.getRoom(spaceId); // Add relevant DMs @@ -528,14 +506,19 @@ export class SpaceStoreClass extends AsyncStoreWithClient { const newPath = new Set(parentPath).add(spaceId); childSpaces.forEach(childSpace => { fn(childSpace.roomId, newPath)?.forEach(roomId => { - roomIds.add(this.findMostUpgradedVersion(roomId, upgradedRoomMap)); + roomIds.add(roomId); }); }); hiddenChildren.get(spaceId)?.forEach(roomId => { - roomIds.add(this.findMostUpgradedVersion(roomId, upgradedRoomMap)); + roomIds.add(roomId); }); - this.spaceFilteredRooms.set(spaceId, roomIds); - return roomIds; + + // Expand room IDs to all known versions of the given rooms + const expandedRoomIds = new Set(Array.from(roomIds).flatMap(roomId => { + return this.matrixClient.getRoomUpgradeHistory(roomId, true).map(r => r.roomId); + })); + this.spaceFilteredRooms.set(spaceId, expandedRoomIds); + return expandedRoomIds; }; fn(s.roomId, new Set()); From 82ad85a9744b0da3f1f3128138ba6064989e15b6 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 8 Oct 2021 10:30:46 +0100 Subject: [PATCH 3/3] Mock usage of getRoomUpgradeHistory in SpaceStore tests --- test/stores/SpaceStore-test.ts | 1 + test/test-utils.js | 1 + 2 files changed, 2 insertions(+) diff --git a/test/stores/SpaceStore-test.ts b/test/stores/SpaceStore-test.ts index e7ca727e28..cdc3e58a4f 100644 --- a/test/stores/SpaceStore-test.ts +++ b/test/stores/SpaceStore-test.ts @@ -77,6 +77,7 @@ describe("SpaceStore", () => { const run = async () => { client.getRoom.mockImplementation(roomId => rooms.find(room => room.roomId === roomId)); + client.getRoomUpgradeHistory.mockImplementation(roomId => [rooms.find(room => room.roomId === roomId)]); await testUtils.setupAsyncStoreWithClient(store, client); jest.runAllTimers(); }; diff --git a/test/test-utils.js b/test/test-utils.js index 2091a6e0ed..d43a08ab3a 100644 --- a/test/test-utils.js +++ b/test/test-utils.js @@ -103,6 +103,7 @@ export function createTestClient() { isUserIgnored: jest.fn().mockReturnValue(false), getCapabilities: jest.fn().mockResolvedValue({}), supportsExperimentalThreads: () => false, + getRoomUpgradeHistory: jest.fn().mockReturnValue([]), }; }