Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix race conditions around threads #8448

Merged
merged 8 commits into from
May 3, 2022
Merged
7 changes: 6 additions & 1 deletion src/components/structures/RoomView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1398,7 +1398,12 @@ export class RoomView extends React.Component<IRoomProps, IRoomState> {
.getServerAggregatedRelation<IThreadBundledRelationship>(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);
}
}
}
}
Expand Down
17 changes: 3 additions & 14 deletions src/components/structures/ThreadPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,6 @@ const ThreadPanel: React.FC<IProps> = ({

const [filterOption, setFilterOption] = useState<ThreadFilterType>(ThreadFilterType.All);
const [room, setRoom] = useState<Room | null>(null);
const [threadCount, setThreadCount] = useState<number>(0);
const [timelineSet, setTimelineSet] = useState<EventTimelineSet | null>(null);
const [narrow, setNarrow] = useState<boolean>(false);

Expand All @@ -206,23 +205,13 @@ const ThreadPanel: React.FC<IProps> = ({
}, [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]);
Expand Down Expand Up @@ -260,7 +249,7 @@ const ThreadPanel: React.FC<IProps> = ({
header={<ThreadPanelHeader
filterOption={filterOption}
setFilterOption={setFilterOption}
empty={threadCount === 0}
empty={!timelineSet?.getLiveTimeline()?.getEvents().length}
/>}
footer={<>
<BetaPill
Expand Down
47 changes: 11 additions & 36 deletions src/components/structures/ThreadView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ limitations under the License.
*/

import React, { createRef, KeyboardEvent } from 'react';
import { Thread, ThreadEvent, THREAD_RELATION_TYPE } from 'matrix-js-sdk/src/models/thread';
import { Thread, THREAD_RELATION_TYPE, ThreadEvent } from 'matrix-js-sdk/src/models/thread';
import { Room } from 'matrix-js-sdk/src/models/room';
import { IEventRelation, MatrixEvent } from 'matrix-js-sdk/src/models/event';
import { TimelineWindow } from 'matrix-js-sdk/src/timeline-window';
Expand Down Expand Up @@ -66,7 +66,6 @@ interface IProps {

interface IState {
thread?: Thread;
lastThreadReply?: MatrixEvent;
layout: Layout;
editState?: EditorStateTransfer;
replyToEvent?: MatrixEvent;
Expand Down Expand Up @@ -104,7 +103,6 @@ export default class ThreadView extends React.Component<IProps, IState> {
}

public componentWillUnmount(): void {
this.teardownThread();
if (this.dispatcherRef) dis.unregister(this.dispatcherRef);
const roomId = this.props.mxEvent.getRoomId();
const room = MatrixClientPeg.get().getRoom(roomId);
Expand All @@ -123,7 +121,6 @@ export default class ThreadView extends React.Component<IProps, IState> {

public componentDidUpdate(prevProps) {
if (prevProps.mxEvent !== this.props.mxEvent) {
this.teardownThread();
this.setupThread(this.props.mxEvent);
}

Expand All @@ -134,7 +131,6 @@ export default class ThreadView extends React.Component<IProps, IState> {

private onAction = (payload: ActionPayload): void => {
if (payload.phase == RightPanelPhases.ThreadView && payload.event) {
this.teardownThread();
this.setupThread(payload.event);
}
switch (payload.action) {
Expand Down Expand Up @@ -164,23 +160,15 @@ export default class ThreadView extends React.Component<IProps, IState> {
};

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);
}
};
Expand All @@ -189,33 +177,15 @@ export default class ThreadView extends React.Component<IProps, IState> {
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()) {
Expand Down Expand Up @@ -298,12 +268,16 @@ export default class ThreadView extends React.Component<IProps, IState> {
};

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,
},
};
}
Expand Down Expand Up @@ -356,6 +330,7 @@ export default class ThreadView extends React.Component<IProps, IState> {
{ this.state.thread && <div className="mx_ThreadView_timelinePanelWrapper">
<FileDropTarget parent={this.card.current} onFileDrop={this.onFileDrop} />
<TimelinePanel
key={this.state?.thread?.id}
ref={this.timelinePanel}
showReadReceipts={false} // Hide the read receipts
// until homeservers speak threads language
Expand Down
10 changes: 6 additions & 4 deletions src/components/views/rooms/EventTile.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -499,16 +499,18 @@ export class UnwrappedEventTile extends React.Component<IProps, IState> {
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 {
Expand Down
6 changes: 2 additions & 4 deletions src/stores/notifications/ThreadsRoomNotificationState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
14 changes: 7 additions & 7 deletions src/utils/EventUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,8 @@ export function isVoiceMessage(mxEvent: MatrixEvent): boolean {
export async function fetchInitialEvent(
client: MatrixClient,
roomId: string,
eventId: string): Promise<MatrixEvent | null> {
eventId: string,
): Promise<MatrixEvent | null> {
let initialEvent: MatrixEvent;

try {
Expand All @@ -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);
}
}

Expand Down
1 change: 1 addition & 0 deletions test/test-utils/test-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
2 changes: 1 addition & 1 deletion test/test-utils/threads.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
};