From f29ef04751c7985af9233458a36e1edd52ea6a68 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Tue, 3 May 2022 14:25:08 +0100 Subject: [PATCH] Fix race conditions around threads (#8448) --- src/components/structures/RoomView.tsx | 7 ++- src/components/structures/ThreadPanel.tsx | 17 ++----- src/components/structures/ThreadView.tsx | 47 +++++-------------- src/components/views/rooms/EventTile.tsx | 10 ++-- .../ThreadsRoomNotificationState.ts | 6 +-- src/utils/EventUtils.ts | 14 +++--- test/test-utils/test-utils.ts | 1 + test/test-utils/threads.ts | 2 +- 8 files changed, 37 insertions(+), 67 deletions(-) diff --git a/src/components/structures/RoomView.tsx b/src/components/structures/RoomView.tsx index 956fee0060..2f55f1b217 100644 --- a/src/components/structures/RoomView.tsx +++ b/src/components/structures/RoomView.tsx @@ -1398,7 +1398,12 @@ export class RoomView extends React.Component { .getServerAggregatedRelation(THREAD_RELATION_TYPE.name); if (!bundledRelationship || event.getThread()) continue; const room = this.context.getRoom(event.getRoomId()); - event.setThread(room.findThreadForEvent(event) ?? room.createThread(event, [], true)); + const thread = room.findThreadForEvent(event); + if (thread) { + event.setThread(thread); + } else { + room.createThread(event.getId(), event, [], true); + } } } } diff --git a/src/components/structures/ThreadPanel.tsx b/src/components/structures/ThreadPanel.tsx index eccb68ed99..3729cfaeaf 100644 --- a/src/components/structures/ThreadPanel.tsx +++ b/src/components/structures/ThreadPanel.tsx @@ -191,7 +191,6 @@ const ThreadPanel: React.FC = ({ const [filterOption, setFilterOption] = useState(ThreadFilterType.All); const [room, setRoom] = useState(null); - const [threadCount, setThreadCount] = useState(0); const [timelineSet, setTimelineSet] = useState(null); const [narrow, setNarrow] = useState(false); @@ -206,23 +205,13 @@ const ThreadPanel: React.FC = ({ }, [mxClient, roomId]); useEffect(() => { - function onNewThread(): void { - setThreadCount(room.threads.size); - } - function refreshTimeline() { - if (timelineSet) timelinePanel.current.refreshTimeline(); + timelinePanel?.current.refreshTimeline(); } - if (room) { - setThreadCount(room.threads.size); - - room.on(ThreadEvent.New, onNewThread); - room.on(ThreadEvent.Update, refreshTimeline); - } + room?.on(ThreadEvent.Update, refreshTimeline); return () => { - room?.removeListener(ThreadEvent.New, onNewThread); room?.removeListener(ThreadEvent.Update, refreshTimeline); }; }, [room, mxClient, timelineSet]); @@ -260,7 +249,7 @@ const ThreadPanel: React.FC = ({ header={} footer={<> { } public componentWillUnmount(): void { - this.teardownThread(); if (this.dispatcherRef) dis.unregister(this.dispatcherRef); const roomId = this.props.mxEvent.getRoomId(); const room = MatrixClientPeg.get().getRoom(roomId); @@ -123,7 +121,6 @@ export default class ThreadView extends React.Component { public componentDidUpdate(prevProps) { if (prevProps.mxEvent !== this.props.mxEvent) { - this.teardownThread(); this.setupThread(this.props.mxEvent); } @@ -134,7 +131,6 @@ export default class ThreadView extends React.Component { private onAction = (payload: ActionPayload): void => { if (payload.phase == RightPanelPhases.ThreadView && payload.event) { - this.teardownThread(); this.setupThread(payload.event); } switch (payload.action) { @@ -164,23 +160,15 @@ export default class ThreadView extends React.Component { }; private setupThread = (mxEv: MatrixEvent) => { - let thread = this.props.room.threads?.get(mxEv.getId()); + let thread = this.props.room.getThread(mxEv.getId()); if (!thread) { - thread = this.props.room.createThread(mxEv, [mxEv], true); + thread = this.props.room.createThread(mxEv.getId(), mxEv, [mxEv], true); } - thread.on(ThreadEvent.Update, this.updateLastThreadReply); this.updateThread(thread); }; - private teardownThread = () => { - if (this.state.thread) { - this.state.thread.removeListener(ThreadEvent.Update, this.updateLastThreadReply); - } - }; - private onNewThread = (thread: Thread) => { if (thread.id === this.props.mxEvent.getId()) { - this.teardownThread(); this.setupThread(this.props.mxEvent); } }; @@ -189,33 +177,15 @@ export default class ThreadView extends React.Component { if (thread && this.state.thread !== thread) { this.setState({ thread, - lastThreadReply: thread.lastReply((ev: MatrixEvent) => { - return ev.isRelation(THREAD_RELATION_TYPE.name) && !ev.status; - }), }, async () => { thread.emit(ThreadEvent.ViewThread); - if (!thread.initialEventsFetched) { - const response = await thread.fetchInitialEvents(); - if (response?.nextBatch) { - this.nextBatch = response.nextBatch; - } - } - + await thread.fetchInitialEvents(); + this.nextBatch = thread.liveTimeline.getPaginationToken(Direction.Backward); this.timelinePanel.current?.refreshTimeline(); }); } }; - private updateLastThreadReply = () => { - if (this.state.thread) { - this.setState({ - lastThreadReply: this.state.thread.lastReply((ev: MatrixEvent) => { - return ev.isRelation(THREAD_RELATION_TYPE.name) && !ev.status; - }), - }); - } - }; - private resetJumpToEvent = (event?: string): void => { if (this.props.initialEvent && this.props.initialEventScrollIntoView && event === this.props.initialEvent?.getId()) { @@ -298,12 +268,16 @@ export default class ThreadView extends React.Component { }; private get threadRelation(): IEventRelation { + const lastThreadReply = this.state.thread?.lastReply((ev: MatrixEvent) => { + return ev.isRelation(THREAD_RELATION_TYPE.name) && !ev.status; + }); + return { "rel_type": THREAD_RELATION_TYPE.name, "event_id": this.state.thread?.id, "is_falling_back": true, "m.in_reply_to": { - "event_id": this.state.lastThreadReply?.getId() ?? this.state.thread?.id, + "event_id": lastThreadReply?.getId() ?? this.state.thread?.id, }, }; } @@ -356,6 +330,7 @@ export default class ThreadView extends React.Component { { this.state.thread &&
{ return null; } + let thread = this.props.mxEvent.getThread(); /** * Accessing the threads value through the room due to a race condition * that will be solved when there are proper backend support for threads * We currently have no reliable way to discover than an event is a thread * when we are at the sync stage */ - const room = MatrixClientPeg.get().getRoom(this.props.mxEvent.getRoomId()); - const thread = room?.threads?.get(this.props.mxEvent.getId()); - - return thread || null; + if (!thread) { + const room = MatrixClientPeg.get().getRoom(this.props.mxEvent.getRoomId()); + thread = room?.findThreadForEvent(this.props.mxEvent); + } + return thread ?? null; } private renderThreadPanelSummary(): JSX.Element | null { diff --git a/src/stores/notifications/ThreadsRoomNotificationState.ts b/src/stores/notifications/ThreadsRoomNotificationState.ts index 6129f141c5..e0ec810cec 100644 --- a/src/stores/notifications/ThreadsRoomNotificationState.ts +++ b/src/stores/notifications/ThreadsRoomNotificationState.ts @@ -31,10 +31,8 @@ export class ThreadsRoomNotificationState extends NotificationState implements I constructor(public readonly room: Room) { super(); - if (this.room?.threads) { - for (const [, thread] of this.room.threads) { - this.onNewThread(thread); - } + for (const thread of this.room.getThreads()) { + this.onNewThread(thread); } this.room.on(ThreadEvent.New, this.onNewThread); } diff --git a/src/utils/EventUtils.ts b/src/utils/EventUtils.ts index 0a860ccf01..bbfaf3f613 100644 --- a/src/utils/EventUtils.ts +++ b/src/utils/EventUtils.ts @@ -217,7 +217,8 @@ export function isVoiceMessage(mxEvent: MatrixEvent): boolean { export async function fetchInitialEvent( client: MatrixClient, roomId: string, - eventId: string): Promise { + eventId: string, +): Promise { let initialEvent: MatrixEvent; try { @@ -228,14 +229,13 @@ export async function fetchInitialEvent( initialEvent = null; } - if (initialEvent?.isThreadRelation && client.supportsExperimentalThreads()) { + if (initialEvent?.isThreadRelation && client.supportsExperimentalThreads() && !initialEvent.getThread()) { + const threadId = initialEvent.threadRootId; + const room = client.getRoom(roomId); try { - const rootEventData = await client.fetchRoomEvent(roomId, initialEvent.threadRootId); - const rootEvent = new MatrixEvent(rootEventData); - const room = client.getRoom(roomId); - room.createThread(rootEvent, [rootEvent], true); + room.createThread(threadId, room.findEventById(threadId), [initialEvent], true); } catch (e) { - logger.warn("Could not find root event: " + initialEvent.threadRootId); + logger.warn("Could not find root event: " + threadId); } } diff --git a/test/test-utils/test-utils.ts b/test/test-utils/test-utils.ts index a590474ffe..fbe7c3aa4a 100644 --- a/test/test-utils/test-utils.ts +++ b/test/test-utils/test-utils.ts @@ -381,6 +381,7 @@ export function mkStubRoom(roomId: string = null, name: string, client: MatrixCl client, myUserId: client?.getUserId(), canInvite: jest.fn(), + getThreads: jest.fn().mockReturnValue([]), } as unknown as Room; } diff --git a/test/test-utils/threads.ts b/test/test-utils/threads.ts index ebed292eb5..8c389d41e1 100644 --- a/test/test-utils/threads.ts +++ b/test/test-utils/threads.ts @@ -92,5 +92,5 @@ export const makeThreadEvents = ({ export const makeThread = (client: MatrixClient, room: Room, props: MakeThreadEventsProps): Thread => { const { rootEvent, events } = makeThreadEvents(props); - return new Thread(rootEvent, { initialEvents: events, room, client }); + return new Thread(rootEvent.getId(), rootEvent, { initialEvents: events, room, client }); };