Skip to content

Commit a7f0ba9

Browse files
author
Germain
authored
Fixes unwanted highlight notifications with encrypted threads (#2862)
1 parent 54d11e1 commit a7f0ba9

File tree

4 files changed

+216
-3
lines changed

4 files changed

+216
-3
lines changed

spec/test-utils/test-utils.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import EventEmitter from "events";
55
import '../olm-loader';
66

77
import { logger } from '../../src/logger';
8-
import { IContent, IEvent, IUnsigned, MatrixEvent, MatrixEventEvent } from "../../src/models/event";
8+
import { IContent, IEvent, IEventRelation, IUnsigned, MatrixEvent, MatrixEventEvent } from "../../src/models/event";
99
import { ClientEvent, EventType, IPusher, MatrixClient, MsgType } from "../../src";
1010
import { SyncState } from "../../src/sync";
1111
import { eventMapperFor } from "../../src/event-mapper";
@@ -78,6 +78,7 @@ interface IEventOpts {
7878
user?: string;
7979
unsigned?: IUnsigned;
8080
redacts?: string;
81+
ts?: number;
8182
}
8283

8384
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
109110
event_id: "$" + testEventIndex++ + "-" + Math.random() + "-" + Math.random(),
110111
txn_id: "~" + Math.random(),
111112
redacts: opts.redacts,
113+
origin_server_ts: opts.ts ?? 0,
112114
};
113115
if (opts.skey !== undefined) {
114116
event.state_key = opts.skey;
@@ -237,11 +239,13 @@ export function mkMembershipCustom<T>(
237239
});
238240
}
239241

240-
interface IMessageOpts {
242+
export interface IMessageOpts {
241243
room?: string;
242244
user: string;
243245
msg?: string;
244246
event?: boolean;
247+
relatesTo?: IEventRelation;
248+
ts?: number;
245249
}
246250

247251
/**
@@ -269,6 +273,10 @@ export function mkMessage(
269273
},
270274
};
271275

276+
if (opts.relatesTo) {
277+
eventOpts.content["m.relates_to"] = opts.relatesTo;
278+
}
279+
272280
if (!eventOpts.content.body) {
273281
eventOpts.content.body = "Random->" + Math.random();
274282
}

spec/test-utils/thread.ts

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
/*
2+
Copyright 2022 The Matrix.org Foundation C.I.C.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
import { RelationType } from "../../src/@types/event";
18+
import { MatrixClient } from "../../src/client";
19+
import { MatrixEvent, MatrixEventEvent } from "../../src/models/event";
20+
import { Room } from "../../src/models/room";
21+
import { Thread } from "../../src/models/thread";
22+
import { mkMessage } from "./test-utils";
23+
24+
export const makeThreadEvent = ({ rootEventId, replyToEventId, ...props }: any & {
25+
rootEventId: string; replyToEventId: string; event?: boolean;
26+
}): MatrixEvent => mkMessage({
27+
...props,
28+
relatesTo: {
29+
event_id: rootEventId,
30+
rel_type: "m.thread",
31+
['m.in_reply_to']: {
32+
event_id: replyToEventId,
33+
},
34+
},
35+
});
36+
37+
type MakeThreadEventsProps = {
38+
roomId: Room["roomId"];
39+
// root message user id
40+
authorId: string;
41+
// user ids of thread replies
42+
// cycled through until thread length is fulfilled
43+
participantUserIds: string[];
44+
// number of messages in the thread, root message included
45+
// optional, default 2
46+
length?: number;
47+
ts?: number;
48+
// provide to set current_user_participated accurately
49+
currentUserId?: string;
50+
};
51+
52+
export const makeThreadEvents = ({
53+
roomId, authorId, participantUserIds, length = 2, ts = 1, currentUserId,
54+
}: MakeThreadEventsProps): { rootEvent: MatrixEvent, events: MatrixEvent[] } => {
55+
const rootEvent = mkMessage({
56+
user: authorId,
57+
room: roomId,
58+
msg: 'root event message ' + Math.random(),
59+
ts,
60+
event: true,
61+
});
62+
63+
const rootEventId = rootEvent.getId();
64+
const events = [rootEvent];
65+
66+
for (let i = 1; i < length; i++) {
67+
const prevEvent = events[i - 1];
68+
const replyToEventId = prevEvent.getId();
69+
const user = participantUserIds[i % participantUserIds.length];
70+
events.push(makeThreadEvent({
71+
user,
72+
room: roomId,
73+
event: true,
74+
msg: `reply ${i} by ${user}`,
75+
rootEventId,
76+
replyToEventId,
77+
// replies are 1ms after each other
78+
ts: ts + i,
79+
}));
80+
}
81+
82+
rootEvent.setUnsigned({
83+
"m.relations": {
84+
[RelationType.Thread]: {
85+
latest_event: events[events.length - 1],
86+
count: length,
87+
current_user_participated: [...participantUserIds, authorId].includes(currentUserId ?? ""),
88+
},
89+
},
90+
});
91+
92+
return { rootEvent, events };
93+
};
94+
95+
type MakeThreadProps = {
96+
room: Room;
97+
client: MatrixClient;
98+
authorId: string;
99+
participantUserIds: string[];
100+
length?: number;
101+
ts?: number;
102+
};
103+
104+
export const mkThread = ({
105+
room,
106+
client,
107+
authorId,
108+
participantUserIds,
109+
length = 2,
110+
ts = 1,
111+
}: MakeThreadProps): { thread: Thread, rootEvent: MatrixEvent, events: MatrixEvent[] } => {
112+
const { rootEvent, events } = makeThreadEvents({
113+
roomId: room.roomId,
114+
authorId,
115+
participantUserIds,
116+
length,
117+
ts,
118+
currentUserId: client.getUserId() ?? "",
119+
});
120+
expect(rootEvent).toBeTruthy();
121+
122+
for (const evt of events) {
123+
room?.reEmitter.reEmit(evt, [
124+
MatrixEventEvent.BeforeRedaction,
125+
]);
126+
}
127+
128+
const thread = room.createThread(rootEvent.getId() ?? "", rootEvent, events, true);
129+
// So that we do not have to mock the thread loading
130+
thread.initialEventsFetched = true;
131+
thread.addEvents(events, true);
132+
133+
return { thread, rootEvent, events };
134+
};

spec/unit/models/thread.spec.ts

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,11 @@ See the License for the specific language governing permissions and
1414
limitations under the License.
1515
*/
1616

17+
import { MatrixClient } from "../../../src/client";
18+
import { Room } from "../../../src/models/room";
1719
import { Thread } from "../../../src/models/thread";
20+
import { mkThread } from "../../test-utils/thread";
21+
import { TestClient } from "../../TestClient";
1822

1923
describe('Thread', () => {
2024
describe("constructor", () => {
@@ -25,4 +29,52 @@ describe('Thread', () => {
2529
}).toThrow("element-web#22141: A thread requires a room in order to function");
2630
});
2731
});
32+
33+
describe("hasUserReadEvent", () => {
34+
const myUserId = "@bob:example.org";
35+
let client: MatrixClient;
36+
let room: Room;
37+
38+
beforeEach(() => {
39+
const testClient = new TestClient(
40+
myUserId,
41+
"DEVICE",
42+
"ACCESS_TOKEN",
43+
undefined,
44+
{ timelineSupport: false },
45+
);
46+
client = testClient.client;
47+
room = new Room("123", client, myUserId);
48+
49+
jest.spyOn(client, "getRoom").mockReturnValue(room);
50+
});
51+
52+
afterAll(() => {
53+
jest.resetAllMocks();
54+
});
55+
56+
it("considers own events with no RR as read", () => {
57+
const { thread, events } = mkThread({
58+
room,
59+
client,
60+
authorId: myUserId,
61+
participantUserIds: [myUserId],
62+
length: 2,
63+
});
64+
65+
expect(thread.hasUserReadEvent(myUserId, events.at(-1)!.getId() ?? "")).toBeTruthy();
66+
});
67+
68+
it("considers other events with no RR as unread", () => {
69+
const { thread, events } = mkThread({
70+
room,
71+
client,
72+
authorId: myUserId,
73+
participantUserIds: ["@alice:example.org"],
74+
length: 2,
75+
});
76+
77+
expect(thread.hasUserReadEvent("@alice:example.org", events.at(-1)!.getId() ?? "")).toBeFalsy();
78+
});
79+
});
2880
});

src/models/thread.ts

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,12 @@ import { RelationType } from "../@types/event";
2222
import { IThreadBundledRelationship, MatrixEvent, MatrixEventEvent } from "./event";
2323
import { EventTimeline } from "./event-timeline";
2424
import { EventTimelineSet, EventTimelineSetHandlerMap } from './event-timeline-set';
25-
import { Room, RoomEvent } from './room';
25+
import { NotificationCountType, Room, RoomEvent } from './room';
2626
import { RoomState } from "./room-state";
2727
import { ServerControlledNamespacedValue } from "../NamespacedValue";
2828
import { logger } from "../logger";
2929
import { ReadReceipt } from "./read-receipt";
30+
import { ReceiptType } from "../@types/read_receipts";
3031

3132
export enum ThreadEvent {
3233
New = "Thread.new",
@@ -417,6 +418,24 @@ export class Thread extends ReadReceipt<EmittedEvents, EventHandlerMap> {
417418
public addReceipt(event: MatrixEvent, synthetic: boolean): void {
418419
throw new Error("Unsupported function on the thread model");
419420
}
421+
422+
public hasUserReadEvent(userId: string, eventId: string): boolean {
423+
if (userId === this.client.getUserId()) {
424+
const publicReadReceipt = this.getReadReceiptForUserId(userId, false, ReceiptType.Read);
425+
const privateReadReceipt = this.getReadReceiptForUserId(userId, false, ReceiptType.ReadPrivate);
426+
const hasUnreads = this.room.getThreadUnreadNotificationCount(this.id, NotificationCountType.Total) > 0;
427+
428+
if (!publicReadReceipt && !privateReadReceipt && !hasUnreads) {
429+
// Consider an event read if it's part of a thread that has no
430+
// read receipts and has no notifications. It is likely that it is
431+
// part of a thread that was created before read receipts for threads
432+
// were supported (via MSC3771)
433+
return true;
434+
}
435+
}
436+
437+
return super.hasUserReadEvent(userId, eventId);
438+
}
420439
}
421440

422441
export const FILTER_RELATED_BY_SENDERS = new ServerControlledNamespacedValue(

0 commit comments

Comments
 (0)