Fixes unwanted highlight notifications with encrypted threads (#2862)

spec/test-utils/test-utils.ts

@@ -5,7 +5,7 @@ import EventEmitter from "events";
 import '../olm-loader';
 
 import { logger } from '../../src/logger';
-import { IContent, IEvent, IUnsigned, MatrixEvent, MatrixEventEvent } from "../../src/models/event";
+import { IContent, IEvent, IEventRelation, IUnsigned, MatrixEvent, MatrixEventEvent } from "../../src/models/event";
 import { ClientEvent, EventType, IPusher, MatrixClient, MsgType } from "../../src";
 import { SyncState } from "../../src/sync";
 import { eventMapperFor } from "../../src/event-mapper";
@@ -78,6 +78,7 @@ interface IEventOpts {
     user?: string;
     unsigned?: IUnsigned;
     redacts?: string;
+    ts?: number;
 }
 
 let testEventIndex = 1; // counter for events, easier for comparison of randomly generated events
@@ -109,6 +110,7 @@ export function mkEvent(opts: IEventOpts & { event?: boolean }, client?: MatrixC
         event_id: "$" + testEventIndex++ + "-" + Math.random() + "-" + Math.random(),
         txn_id: "~" + Math.random(),
         redacts: opts.redacts,
+        origin_server_ts: opts.ts ?? 0,
     };
     if (opts.skey !== undefined) {
         event.state_key = opts.skey;
@@ -237,11 +239,13 @@ export function mkMembershipCustom<T>(
     });
 }
 
-interface IMessageOpts {
+export interface IMessageOpts {
     room?: string;
     user: string;
     msg?: string;
     event?: boolean;
+    relatesTo?: IEventRelation;
+    ts?: number;
 }
 
 /**
@@ -269,6 +273,10 @@ export function mkMessage(
         },
     };
 
+    if (opts.relatesTo) {
+        eventOpts.content["m.relates_to"] = opts.relatesTo;
+    }
+
     if (!eventOpts.content.body) {
         eventOpts.content.body = "Random->" + Math.random();
     }

spec/test-utils/thread.ts

@@ -0,0 +1,134 @@
+/*
+Copyright 2022 The Matrix.org Foundation C.I.C.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+import { RelationType } from "../../src/@types/event";
+import { MatrixClient } from "../../src/client";
+import { MatrixEvent, MatrixEventEvent } from "../../src/models/event";
+import { Room } from "../../src/models/room";
+import { Thread } from "../../src/models/thread";
+import { mkMessage } from "./test-utils";
+
+export const makeThreadEvent = ({ rootEventId, replyToEventId, ...props }: any & {
+    rootEventId: string; replyToEventId: string; event?: boolean;
+}): MatrixEvent => mkMessage({
+    ...props,
+    relatesTo: {
+        event_id: rootEventId,
+        rel_type: "m.thread",
+        ['m.in_reply_to']: {
+            event_id: replyToEventId,
+        },
+    },
+});
+
+type MakeThreadEventsProps = {
+    roomId: Room["roomId"];
+    // root message user id
+    authorId: string;
+    // user ids of thread replies
+    // cycled through until thread length is fulfilled
+    participantUserIds: string[];
+    // number of messages in the thread, root message included
+    // optional, default 2
+    length?: number;
+    ts?: number;
+    // provide to set current_user_participated accurately
+    currentUserId?: string;
+};
+
+export const makeThreadEvents = ({
+    roomId, authorId, participantUserIds, length = 2, ts = 1, currentUserId,
+}: MakeThreadEventsProps): { rootEvent: MatrixEvent, events: MatrixEvent[] } => {
+    const rootEvent = mkMessage({
+        user: authorId,
+        room: roomId,
+        msg: 'root event message ' + Math.random(),
+        ts,
+        event: true,
+    });
+
+    const rootEventId = rootEvent.getId();
+    const events = [rootEvent];
+
+    for (let i = 1; i < length; i++) {
+        const prevEvent = events[i - 1];
+        const replyToEventId = prevEvent.getId();
+        const user = participantUserIds[i % participantUserIds.length];
+        events.push(makeThreadEvent({
+            user,
+            room: roomId,
+            event: true,
+            msg: `reply ${i} by ${user}`,
+            rootEventId,
+            replyToEventId,
+            // replies are 1ms after each other
+            ts: ts + i,
+        }));
+    }
+
+    rootEvent.setUnsigned({
+        "m.relations": {
+            [RelationType.Thread]: {
+                latest_event: events[events.length - 1],
+                count: length,
+                current_user_participated: [...participantUserIds, authorId].includes(currentUserId ?? ""),
+            },
+        },
+    });
+
+    return { rootEvent, events };
+};
+
+type MakeThreadProps = {
+    room: Room;
+    client: MatrixClient;
+    authorId: string;
+    participantUserIds: string[];
+    length?: number;
+    ts?: number;
+};
+
+export const mkThread = ({
+    room,
+    client,
+    authorId,
+    participantUserIds,
+    length = 2,
+    ts = 1,
+}: MakeThreadProps): { thread: Thread, rootEvent: MatrixEvent, events: MatrixEvent[] } => {
+    const { rootEvent, events } = makeThreadEvents({
+        roomId: room.roomId,
+        authorId,
+        participantUserIds,
+        length,
+        ts,
+        currentUserId: client.getUserId() ?? "",
+    });
+    expect(rootEvent).toBeTruthy();
+
+    for (const evt of events) {
+        room?.reEmitter.reEmit(evt, [
+            MatrixEventEvent.BeforeRedaction,
+        ]);
+    }
+
+    const thread = room.createThread(rootEvent.getId() ?? "", rootEvent, events, true);
+    // So that we do not have to mock the thread loading
+    thread.initialEventsFetched = true;
+    thread.addEvents(events, true);
+
+    return { thread, rootEvent, events };
+};

spec/unit/models/thread.spec.ts

@@ -14,7 +14,11 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
+import { MatrixClient } from "../../../src/client";
+import { Room } from "../../../src/models/room";
 import { Thread } from "../../../src/models/thread";
+import { mkThread } from "../../test-utils/thread";
+import { TestClient } from "../../TestClient";
 
 describe('Thread', () => {
     describe("constructor", () => {
@@ -25,4 +29,52 @@ describe('Thread', () => {
             }).toThrow("element-web#22141: A thread requires a room in order to function");
         });
     });
+
+    describe("hasUserReadEvent", () => {
+        const myUserId = "@bob:example.org";
+        let client: MatrixClient;
+        let room: Room;
+
+        beforeEach(() => {
+            const testClient = new TestClient(
+                myUserId,
+                "DEVICE",
+                "ACCESS_TOKEN",
+                undefined,
+                { timelineSupport: false },
+            );
+            client = testClient.client;
+            room = new Room("123", client, myUserId);
+
+            jest.spyOn(client, "getRoom").mockReturnValue(room);
+        });
+
+        afterAll(() => {
+            jest.resetAllMocks();
+        });
+
+        it("considers own events with no RR as read", () => {
+            const { thread, events } = mkThread({
+                room,
+                client,
+                authorId: myUserId,
+                participantUserIds: [myUserId],
+                length: 2,
+            });
+
+            expect(thread.hasUserReadEvent(myUserId, events.at(-1)!.getId() ?? "")).toBeTruthy();
+        });
+
+        it("considers other events with no RR as unread", () => {
+            const { thread, events } = mkThread({
+                room,
+                client,
+                authorId: myUserId,
+                participantUserIds: ["@alice:example.org"],
+                length: 2,
+            });
+
+            expect(thread.hasUserReadEvent("@alice:example.org", events.at(-1)!.getId() ?? "")).toBeFalsy();
+        });
+    });
 });

src/models/thread.ts

@@ -22,11 +22,12 @@ import { RelationType } from "../@types/event";
 import { IThreadBundledRelationship, MatrixEvent, MatrixEventEvent } from "./event";
 import { EventTimeline } from "./event-timeline";
 import { EventTimelineSet, EventTimelineSetHandlerMap } from './event-timeline-set';
-import { Room, RoomEvent } from './room';
+import { NotificationCountType, Room, RoomEvent } from './room';
 import { RoomState } from "./room-state";
 import { ServerControlledNamespacedValue } from "../NamespacedValue";
 import { logger } from "../logger";
 import { ReadReceipt } from "./read-receipt";
+import { ReceiptType } from "../@types/read_receipts";
 
 export enum ThreadEvent {
     New = "Thread.new",
@@ -417,6 +418,24 @@ export class Thread extends ReadReceipt<EmittedEvents, EventHandlerMap> {
     public addReceipt(event: MatrixEvent, synthetic: boolean): void {
         throw new Error("Unsupported function on the thread model");
     }
+
+    public hasUserReadEvent(userId: string, eventId: string): boolean {
+        if (userId === this.client.getUserId()) {
+            const publicReadReceipt = this.getReadReceiptForUserId(userId, false, ReceiptType.Read);
+            const privateReadReceipt = this.getReadReceiptForUserId(userId, false, ReceiptType.ReadPrivate);
+            const hasUnreads = this.room.getThreadUnreadNotificationCount(this.id, NotificationCountType.Total) > 0;
+
+            if (!publicReadReceipt && !privateReadReceipt && !hasUnreads) {
+                // Consider an event read if it's part of a thread that has no
+                // read receipts and has no notifications. It is likely that it is
+                // part of a thread that was created before read receipts for threads
+                // were supported (via MSC3771)
+                return true;
+            }
+        }
+
+        return super.hasUserReadEvent(userId, eventId);
+    }
 }
 
 export const FILTER_RELATED_BY_SENDERS = new ServerControlledNamespacedValue(