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

Fixes unwanted highlight notifications with encrypted threads #2862

Merged
merged 1 commit into from
Nov 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions spec/test-utils/test-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}

/**
Expand Down Expand Up @@ -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();
}
Expand Down
134 changes: 134 additions & 0 deletions spec/test-utils/thread.ts
Original file line number Diff line number Diff line change
@@ -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 };
};
52 changes: 52 additions & 0 deletions spec/unit/models/thread.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand All @@ -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();
});
});
});
21 changes: 20 additions & 1 deletion src/models/thread.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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(
Expand Down