Improve hasUserReadEvent and getUserReadUpTo realibility with threads

spec/unit/models/thread.spec.ts

@@ -116,7 +116,7 @@ describe("Thread", () => {
                     // last threaded receipt
                     "$event2:localhost": {
                         [ReceiptType.Read]: {
-                            [client.getUserId()!]: { ts: 300 },
+                            [client.getUserId()!]: { ts: 300, thread_id: "$threadId" },
                         },
                     },
                 },
@@ -137,9 +137,9 @@ describe("Thread", () => {
                 authorId: myUserId,
                 participantUserIds: [myUserId],
                 length: 2,
-                ts: 201,
             });
 
+            // The event is automatically considered read as the current user is the sender
             expect(thread.hasUserReadEvent(myUserId, events.at(-1)!.getId() ?? "")).toBeTruthy();
         });
 
@@ -267,8 +267,7 @@ describe("Thread", () => {
                 ts: 10,
             });
 
-            // The 10th event has been read, as alice's last unthreaded receipt is at ts 200
-            // and `mkThread` increment every thread response by 1ms.
+            // This is marked as read as it is before alice's first threaded receipt...
             expect(thread.getEventReadUpTo(myUserId)).toBe(events.at(-1)!.getId());
 
             const { thread: thread2 } = mkThread({

src/client.ts

@@ -601,9 +601,9 @@ interface IJoinRequestBody {
     third_party_signed?: IThirdPartySigned;
 }
 
-export interface ITagMetadata {
+interface ITagMetadata {
     [key: string]: any;
-    order?: number;
+    order: number;
 }
 
 interface IMessagesResponse {

src/models/room.ts

@@ -310,9 +310,12 @@ export class Room extends ReadReceipt<RoomEmittedEvents, RoomEventHandlerMap> {
     private readonly threadNotifications = new Map<string, NotificationCount>();
     public readonly cachedThreadReadReceipts = new Map<string, CachedReceiptStructure[]>();
     // Useful to know at what point the current user has started using threads in this room
-    public oldestThreadedReceiptTs = Infinity;
-    // Important to compute `hasUserReadEvent` and similar methods correctly.
-    public unthreadedReceipts = new Map<string, Receipt>();
+    private oldestThreadedReceiptTs = Infinity;
+    /**
+     * Keeping a record of the lastest unthread receipts per user
+     * This is useful in determining whether a user has read a thread or not
+     */
+    private unthreadedReceipts = new Map<string, Receipt>();
     private readonly timelineSets: EventTimelineSet[];
     public readonly threadsTimelineSets: EventTimelineSet[] = [];
     // any filtered timeline sets we're maintaining for this room
@@ -3292,6 +3295,24 @@ export class Room extends ReadReceipt<RoomEmittedEvents, RoomEventHandlerMap> {
         }
         event.applyVisibilityEvent(visibilityChange);
     }
+
+    /**
+     * Find when a client has gained thread capabilities by inspecting the oldest
+     * threaded receipt
+     * @returns the timestamp of the oldest threaded receipt
+     */
+    public getOldestThreadedReceiptTs(): number {
+        return this.oldestThreadedReceiptTs;
+    }
+
+    /**
+     * Returns the most receipt unthreaded receipt for a given user
+     * @param userId - the MxID of the User
+     * @returns an unthreaded Receipt
+     */
+    public getLastUnthreadedReceiptFor(userId: string): Receipt | undefined {
+        return this.unthreadedReceipts.get(userId);
+    }
 }
 
 // a map from current event status to a list of allowed next statuses

src/models/thread.ts

@@ -504,41 +504,69 @@ export class Thread extends ReadReceipt<EmittedEvents, EventHandlerMap> {
         throw new Error("Unsupported function on the thread model");
     }
 
+    /**
+     * Get the ID of the event that a given user has read up to, or null if we
+     * have received no read receipts from them.
+     * @param userId - The user ID to get read receipt event ID for
+     * @param ignoreSynthesized - If true, return only receipts that have been
+     *                            sent by the server, not implicit ones generated
+     *                            by the JS SDK.
+     * @returns ID of the latest event that the given user has read, or null.
+     */
     public getEventReadUpTo(userId: string, ignoreSynthesized?: boolean): string | null {
         const isCurrentUser = userId === this.client.getUserId();
-        if (isCurrentUser && this.lastReply()) {
+        const lastReply = this.lastReply();
+        if (isCurrentUser && lastReply) {
             // If the last activity in a thread is prior to the first threaded read receipt
-            // sent in the room we want to consider this thread as read.
-            const beforeFirstThreadedReceipt = (this.lastReply()?.getTs() ?? 0) < this.room.oldestThreadedReceiptTs;
+            // sent in the room (suggesting that it was sent before the user started
+            // using a client that supported threaded read receipts), we want to
+            // consider this thread as read.
+            const beforeFirstThreadedReceipt = (lastReply.getTs() ?? 0) < this.room.getOldestThreadedReceiptTs();
             if (beforeFirstThreadedReceipt) {
                 return this.timeline.at(-1)?.getId() ?? null;
             }
         }
 
         const readUpToId = super.getEventReadUpTo(userId, ignoreSynthesized);
 
-        // Checking whether the unthreaded read receipt for that user is more recent
+        // Check whether the unthreaded read receipt for that user is more recent
         // than the read receipt inside that thread.
         if (this.lastReply()) {
-            if (!this.room.unthreadedReceipts.has(userId)) {
+            const unthreadedReceipt = this.room.getLastUnthreadedReceiptFor(userId);
+            if (!unthreadedReceipt) {
                 return readUpToId;
             }
 
-            const unthreadedReceiptTs = this.room.unthreadedReceipts.get(userId)!.ts;
             for (let i = this.timeline?.length - 1; i >= 0; --i) {
                 const ev = this.timeline[i];
+                // If we encounter the `readUpToId` we do not need to look further
+                // there is no "more recent" unthreaded read receipt
                 if (ev.getId() === readUpToId) return readUpToId;
-                if (ev.getTs() < unthreadedReceiptTs) return ev.getId() ?? readUpToId;
+
+                // Inspecting events from most recent to oldest, we're checking
+                // whether an unthreaded read receipt is more recent that the current event.
+                // We usually prefer relying on the order of the DAG but in this scenario
+                // it is not possible and we have to rely on timestamp
+                if (ev.getTs() < unthreadedReceipt.ts) return ev.getId() ?? readUpToId;
             }
         }
 
         return readUpToId;
     }
 
+    /**
+     * Determines if the given user has read a particular event ID with the known
+     * history of the room. This is not a definitive check as it relies only on
+     * what is available to the thread at the time of execution.
+     * @param userId - The user ID to check the read state of.
+     * @param eventId - The event ID to check if the user read.
+     * @returns True if the user has read the event, false otherwise.
+     */
     public hasUserReadEvent(userId: string, eventId: string): boolean {
         if (userId === this.client.getUserId()) {
-            const beforeFirstThreadedReceipt = (this.lastReply()?.getTs() ?? 0) < this.room.oldestThreadedReceiptTs;
-            const unthreadedReceiptTs = this.room.unthreadedReceipts.get(userId)?.ts ?? 0;
+            const beforeFirstThreadedReceipt =
+                (this.lastReply()?.getTs() ?? 0) < this.room.getOldestThreadedReceiptTs();
+            const unthreadedReceiptTs = this.room.getLastUnthreadedReceiptFor(userId)?.ts ?? 0;
             const beforeLastUnthreadedReceipt = (this?.lastReply()?.getTs() ?? 0) < unthreadedReceiptTs;
             if (beforeFirstThreadedReceipt || beforeLastUnthreadedReceipt) {
                 return true;