From a7f0ba97cdcea8792d7460b32f7fdd9239233ece Mon Sep 17 00:00:00 2001 From: Germain Date: Fri, 11 Nov 2022 09:27:16 +0000 Subject: [PATCH] Fixes unwanted highlight notifications with encrypted threads (#2862) --- spec/test-utils/test-utils.ts | 12 ++- spec/test-utils/thread.ts | 134 ++++++++++++++++++++++++++++++++ spec/unit/models/thread.spec.ts | 52 +++++++++++++ src/models/thread.ts | 21 ++++- 4 files changed, 216 insertions(+), 3 deletions(-) create mode 100644 spec/test-utils/thread.ts diff --git a/spec/test-utils/test-utils.ts b/spec/test-utils/test-utils.ts index 00a4d656d19..af87ebbe64f 100644 --- a/spec/test-utils/test-utils.ts +++ b/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( }); } -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(); } diff --git a/spec/test-utils/thread.ts b/spec/test-utils/thread.ts new file mode 100644 index 00000000000..376b8d81121 --- /dev/null +++ b/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 }; +}; diff --git a/spec/unit/models/thread.spec.ts b/spec/unit/models/thread.spec.ts index 109af08a33b..37e77954795 100644 --- a/spec/unit/models/thread.spec.ts +++ b/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(); + }); + }); }); diff --git a/src/models/thread.ts b/src/models/thread.ts index 384500aed4f..3c02558bf34 100644 --- a/src/models/thread.ts +++ b/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 { 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(