From c49a527e5ee24c3ba1216c464653ef3ec05812c4 Mon Sep 17 00:00:00 2001 From: Andy Balaam Date: Tue, 28 Nov 2023 14:43:48 +0000 Subject: [PATCH] Rewrite receipt-handling code (#3901) * Rewrite receipt-handling code * Add tests around dangling receipts * Fix mark as read for some rooms * Add missing word --------- Co-authored-by: Florian Duros Co-authored-by: Florian Duros --- spec/test-utils/thread.ts | 20 + spec/unit/models/room-receipts.spec.ts | 541 +++++++++++++++++++++++++ spec/unit/models/thread.spec.ts | 101 +++-- spec/unit/read-receipt.spec.ts | 15 + spec/unit/room.spec.ts | 63 ++- src/client.ts | 4 +- src/models/compare-event-ordering.ts | 139 +++++++ src/models/event-timeline-set.ts | 20 +- src/models/read-receipt.ts | 60 ++- src/models/room-receipts.ts | 429 ++++++++++++++++++++ src/models/room.ts | 50 +++ src/models/thread.ts | 23 +- 12 files changed, 1385 insertions(+), 80 deletions(-) create mode 100644 spec/unit/models/room-receipts.spec.ts create mode 100644 src/models/compare-event-ordering.ts create mode 100644 src/models/room-receipts.ts diff --git a/spec/test-utils/thread.ts b/spec/test-utils/thread.ts index 2b84239d3..50d768863 100644 --- a/spec/test-utils/thread.ts +++ b/spec/test-utils/thread.ts @@ -161,3 +161,23 @@ export const mkThread = ({ return { thread, rootEvent, events }; }; + +/** + * Create a thread, and make sure the events are added to the thread and the + * room's timeline as if they came in via sync. + * + * Note that mkThread doesn't actually add the events properly to the room. + */ +export const populateThread = ({ + room, + client, + authorId, + participantUserIds, + length = 2, + ts = 1, +}: MakeThreadProps): MakeThreadResult => { + const ret = mkThread({ room, client, authorId, participantUserIds, length, ts }); + ret.thread.initialEventsFetched = true; + room.addLiveEvents(ret.events); + return ret; +}; diff --git a/spec/unit/models/room-receipts.spec.ts b/spec/unit/models/room-receipts.spec.ts new file mode 100644 index 000000000..d11b16d03 --- /dev/null +++ b/spec/unit/models/room-receipts.spec.ts @@ -0,0 +1,541 @@ +/* +Copyright 2023 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 { FeatureSupport, MatrixClient, MatrixEvent, ReceiptContent, THREAD_RELATION_TYPE, Thread } from "../../../src"; +import { Room } from "../../../src/models/room"; + +/** + * Note, these tests check the functionality of the RoomReceipts class, but most + * of them access that functionality via the surrounding Room class, because a + * room is required for RoomReceipts to function, and this matches the pattern + * of how this code is used in the wild. + */ +describe("RoomReceipts", () => { + beforeAll(() => { + jest.replaceProperty(Thread, "hasServerSideSupport", FeatureSupport.Stable); + }); + + afterAll(() => { + jest.restoreAllMocks(); + }); + + it("reports events unread if there are no receipts", () => { + // Given there are no receipts in the room + const room = createRoom(); + const [event] = createEvent(); + room.addLiveEvents([event]); + + // When I ask about any event, then it is unread + expect(room.hasUserReadEvent(readerId, event.getId()!)).toBe(false); + }); + + it("reports events we sent as read even if there are no (real) receipts", () => { + // Given there are no receipts in the room + const room = createRoom(); + const [event] = createEventSentBy(readerId); + room.addLiveEvents([event]); + + // When I ask about an event I sent, it is read (because a synthetic + // receipt was created and stored in RoomReceipts) + expect(room.hasUserReadEvent(readerId, event.getId()!)).toBe(true); + }); + + it("reports read if we receive an unthreaded receipt for this event", () => { + // Given my event exists and is unread + const room = createRoom(); + const [event, eventId] = createEvent(); + room.addLiveEvents([event]); + expect(room.hasUserReadEvent(readerId, eventId)).toBe(false); + + // When we receive a receipt for this event+user + room.addReceipt(createReceipt(readerId, event)); + + // Then that event is read + expect(room.hasUserReadEvent(readerId, eventId)).toBe(true); + }); + + it("reports read if we receive an unthreaded receipt for a later event", () => { + // Given we have 2 events + const room = createRoom(); + const [event1, event1Id] = createEvent(); + const [event2] = createEvent(); + room.addLiveEvents([event1, event2]); + + // When we receive a receipt for the later event + room.addReceipt(createReceipt(readerId, event2)); + + // Then the earlier one is read + expect(room.hasUserReadEvent(readerId, event1Id)).toBe(true); + }); + + it("reports read for a non-live event if we receive an unthreaded receipt for a live one", () => { + // Given we have 2 events: one live and one old + const room = createRoom(); + const [oldEvent, oldEventId] = createEvent(); + const [liveEvent] = createEvent(); + room.addLiveEvents([liveEvent]); + createOldTimeline(room, [oldEvent]); + + // When we receive a receipt for the live event + room.addReceipt(createReceipt(readerId, liveEvent)); + + // Then the earlier one is read + expect(room.hasUserReadEvent(readerId, oldEventId)).toBe(true); + }); + + it("compares by timestamp if two events are in separate old timelines", () => { + // Given we have 2 events, both in old timelines, with event2 after + // event1 in terms of timestamps + const room = createRoom(); + const [event1, event1Id] = createEvent(); + const [event2, event2Id] = createEvent(); + event1.event.origin_server_ts = 1; + event2.event.origin_server_ts = 2; + createOldTimeline(room, [event1]); + createOldTimeline(room, [event2]); + + // When we receive a receipt for the older event + room.addReceipt(createReceipt(readerId, event1)); + + // Then the earlier one is read and the later one is not + expect(room.hasUserReadEvent(readerId, event1Id)).toBe(true); + expect(room.hasUserReadEvent(readerId, event2Id)).toBe(false); + }); + + it("reports unread if we receive an unthreaded receipt for an earlier event", () => { + // Given we have 2 events + const room = createRoom(); + const [event1] = createEvent(); + const [event2, event2Id] = createEvent(); + room.addLiveEvents([event1, event2]); + + // When we receive a receipt for the earlier event + room.addReceipt(createReceipt(readerId, event1)); + + // Then the later one is unread + expect(room.hasUserReadEvent(readerId, event2Id)).toBe(false); + }); + + it("reports unread if we receive an unthreaded receipt for a different user", () => { + // Given my event exists and is unread + const room = createRoom(); + const [event, eventId] = createEvent(); + room.addLiveEvents([event]); + expect(room.hasUserReadEvent(readerId, eventId)).toBe(false); + + // When we receive a receipt for another user + room.addReceipt(createReceipt(otherUserId, event)); + + // Then the event is still unread since the receipt was not for us + expect(room.hasUserReadEvent(readerId, eventId)).toBe(false); + + // But it's read for the other person + expect(room.hasUserReadEvent(otherUserId, eventId)).toBe(true); + }); + + it("reports events we sent as read even if an earlier receipt arrives", () => { + // Given we sent an event after some other event + const room = createRoom(); + const [previousEvent] = createEvent(); + const [myEvent] = createEventSentBy(readerId); + room.addLiveEvents([previousEvent, myEvent]); + + // And I just received a receipt for the previous event + room.addReceipt(createReceipt(readerId, previousEvent)); + + // When I ask about the event I sent, it is read (because of synthetic receipts) + expect(room.hasUserReadEvent(readerId, myEvent.getId()!)).toBe(true); + }); + + it("considers events after ones we sent to be unread", () => { + // Given we sent an event, then another event came in + const room = createRoom(); + const [myEvent] = createEventSentBy(readerId); + const [laterEvent] = createEvent(); + room.addLiveEvents([myEvent, laterEvent]); + + // When I ask about the later event, it is unread (because it's after the synthetic receipt) + expect(room.hasUserReadEvent(readerId, laterEvent.getId()!)).toBe(false); + }); + + it("correctly reports readness even when receipts arrive out of order", () => { + // Given we have 3 events + const room = createRoom(); + const [event1] = createEvent(); + const [event2, event2Id] = createEvent(); + const [event3, event3Id] = createEvent(); + room.addLiveEvents([event1, event2, event3]); + + // When we receive receipts for the older events out of order + room.addReceipt(createReceipt(readerId, event2)); + room.addReceipt(createReceipt(readerId, event1)); + + // Then we correctly ignore the older receipt + expect(room.hasUserReadEvent(readerId, event2Id)).toBe(true); + expect(room.hasUserReadEvent(readerId, event3Id)).toBe(false); + }); + + it("reports read if we receive a threaded receipt for this event (main)", () => { + // Given my event exists and is unread + const room = createRoom(); + const [event, eventId] = createEvent(); + room.addLiveEvents([event]); + expect(room.hasUserReadEvent(readerId, eventId)).toBe(false); + + // When we receive a receipt for this event+user + room.addReceipt(createThreadedReceipt(readerId, event, "main")); + + // Then that event is read + expect(room.hasUserReadEvent(readerId, eventId)).toBe(true); + }); + + it("reports read if we receive a threaded receipt for this event (non-main)", () => { + // Given my event exists and is unread + const room = createRoom(); + const [root, rootId] = createEvent(); + const [event, eventId] = createThreadedEvent(root); + setupThread(room, root); + room.addLiveEvents([root, event]); + expect(room.hasUserReadEvent(readerId, eventId)).toBe(false); + + // When we receive a receipt for this event on this thread + room.addReceipt(createThreadedReceipt(readerId, event, rootId)); + + // Then that event is read + expect(room.hasUserReadEvent(readerId, eventId)).toBe(true); + }); + + it("reports read if we receive an threaded receipt for a later event", () => { + // Given we have 2 events in a thread + const room = createRoom(); + const [root, rootId] = createEvent(); + const [event1, event1Id] = createThreadedEvent(root); + const [event2] = createThreadedEvent(root); + setupThread(room, root); + room.addLiveEvents([root, event1, event2]); + + // When we receive a receipt for the later event + room.addReceipt(createThreadedReceipt(readerId, event2, rootId)); + + // Then the earlier one is read + expect(room.hasUserReadEvent(readerId, event1Id)).toBe(true); + }); + + it("reports unread if we receive an threaded receipt for an earlier event", () => { + // Given we have 2 events in a thread + const room = createRoom(); + const [root, rootId] = createEvent(); + const [event1] = createThreadedEvent(root); + const [event2, event2Id] = createThreadedEvent(root); + setupThread(room, root); + room.addLiveEvents([root, event1, event2]); + + // When we receive a receipt for the earlier event + room.addReceipt(createThreadedReceipt(readerId, event1, rootId)); + + // Then the later one is unread + expect(room.hasUserReadEvent(readerId, event2Id)).toBe(false); + }); + + it("reports unread if we receive an threaded receipt for a different user", () => { + // Given my event exists and is unread + const room = createRoom(); + const [root, rootId] = createEvent(); + const [event, eventId] = createThreadedEvent(root); + setupThread(room, root); + room.addLiveEvents([root, event]); + expect(room.hasUserReadEvent(readerId, eventId)).toBe(false); + + // When we receive a receipt for another user + room.addReceipt(createThreadedReceipt(otherUserId, event, rootId)); + + // Then the event is still unread since the receipt was not for us + expect(room.hasUserReadEvent(readerId, eventId)).toBe(false); + + // But it's read for the other person + expect(room.hasUserReadEvent(otherUserId, eventId)).toBe(true); + }); + + it("reports unread if we receive a receipt for a later event in a different thread", () => { + // Given 2 events exist in different threads + const room = createRoom(); + const [root1] = createEvent(); + const [root2] = createEvent(); + const [thread1, thread1Id] = createThreadedEvent(root1); + const [thread2] = createThreadedEvent(root2); + setupThread(room, root1); + setupThread(room, root2); + room.addLiveEvents([root1, root2, thread1, thread2]); + + // When we receive a receipt for the later event + room.addReceipt(createThreadedReceipt(readerId, thread2, root2.getId()!)); + + // Then the old one is still unread since the receipt was not for this thread + expect(room.hasUserReadEvent(readerId, thread1Id)).toBe(false); + }); + + it("correctly reports readness even when threaded receipts arrive out of order", () => { + // Given we have 3 events + const room = createRoom(); + const [root, rootId] = createEvent(); + const [event1] = createThreadedEvent(root); + const [event2, event2Id] = createThreadedEvent(root); + const [event3, event3Id] = createThreadedEvent(root); + setupThread(room, root); + room.addLiveEvents([root, event1, event2, event3]); + + // When we receive receipts for the older events out of order + room.addReceipt(createThreadedReceipt(readerId, event2, rootId)); + room.addReceipt(createThreadedReceipt(readerId, event1, rootId)); + + // Then we correctly ignore the older receipt + expect(room.hasUserReadEvent(readerId, event2Id)).toBe(true); + expect(room.hasUserReadEvent(readerId, event3Id)).toBe(false); + }); + + it("correctly reports readness when mixing threaded and unthreaded receipts", () => { + // Given we have a setup from this presentation: + // https://docs.google.com/presentation/d/1H1gxRmRFAm8d71hCILWmpOYezsvdlb7cB6ANl-20Gns/edit?usp=sharing + // + // Main1----\ + // | ---Thread1a <- threaded receipt + // | | + // | Thread1b + // threaded receipt -> Main2--\ + // | ----------------Thread2a <- unthreaded receipt + // Main3 | + // Thread2b <- threaded receipt + // + const room = createRoom(); + const [main1, main1Id] = createEvent(); + const [main2, main2Id] = createEvent(); + const [main3, main3Id] = createEvent(); + const [thread1a, thread1aId] = createThreadedEvent(main1); + const [thread1b, thread1bId] = createThreadedEvent(main1); + const [thread2a, thread2aId] = createThreadedEvent(main2); + const [thread2b, thread2bId] = createThreadedEvent(main2); + setupThread(room, main1); + setupThread(room, main2); + room.addLiveEvents([main1, thread1a, thread1b, main2, thread2a, main3, thread2b]); + + // And the timestamps on the events are consistent with the order above + main1.event.origin_server_ts = 1; + thread1a.event.origin_server_ts = 2; + thread1b.event.origin_server_ts = 3; + main2.event.origin_server_ts = 4; + thread2a.event.origin_server_ts = 5; + main3.event.origin_server_ts = 6; + thread2b.event.origin_server_ts = 7; + // (Note: in principle, we have the information needed to order these + // events without using their timestamps, since they all came in via + // addLiveEvents. In reality, some of them would have come in via the + // /relations API, making it impossible to get the correct ordering + // without MSC4033, which is why we fall back to timestamps. I.e. we + // definitely could fix the code to make the above + // timestamp-manipulation unnecessary, but it would only make this test + // neater, not actually help in the real world.) + + // When the receipts arrive + room.addReceipt(createThreadedReceipt(readerId, main2, "main")); + room.addReceipt(createThreadedReceipt(readerId, thread1a, main1Id)); + room.addReceipt(createReceipt(readerId, thread2a)); + room.addReceipt(createThreadedReceipt(readerId, thread2b, main2Id)); + + // Then we correctly identify that only main3 is unread + expect(room.hasUserReadEvent(readerId, main1Id)).toBe(true); + expect(room.hasUserReadEvent(readerId, main2Id)).toBe(true); + expect(room.hasUserReadEvent(readerId, main3Id)).toBe(false); + expect(room.hasUserReadEvent(readerId, thread1aId)).toBe(true); + expect(room.hasUserReadEvent(readerId, thread1bId)).toBe(true); + expect(room.hasUserReadEvent(readerId, thread2aId)).toBe(true); + expect(room.hasUserReadEvent(readerId, thread2bId)).toBe(true); + }); + + describe("dangling receipts", () => { + it("reports unread if the unthreaded receipt is in a dangling state", () => { + const room = createRoom(); + const [event, eventId] = createEvent(); + // When we receive a receipt for this event+user + room.addReceipt(createReceipt(readerId, event)); + + // The event is not added in the room + // So the receipt is in a dangling state + expect(room.hasUserReadEvent(readerId, eventId)).toBe(false); + + // Add the event to the room + // The receipt is removed from the dangling state + room.addLiveEvents([event]); + + // Then the event is read + expect(room.hasUserReadEvent(readerId, eventId)).toBe(true); + }); + + it("reports unread if the threaded receipt is in a dangling state", () => { + const room = createRoom(); + const [root, rootId] = createEvent(); + const [event, eventId] = createThreadedEvent(root); + setupThread(room, root); + + // When we receive a receipt for this event+user + room.addReceipt(createThreadedReceipt(readerId, event, rootId)); + + // The event is not added in the room + // So the receipt is in a dangling state + expect(room.hasUserReadEvent(readerId, eventId)).toBe(false); + + // Add the events to the room + // The receipt is removed from the dangling state + room.addLiveEvents([root, event]); + + // Then the event is read + expect(room.hasUserReadEvent(readerId, eventId)).toBe(true); + }); + + it("should handle multiple dangling receipts for the same event", () => { + const room = createRoom(); + const [event, eventId] = createEvent(); + // When we receive a receipt for this event+user + room.addReceipt(createReceipt(readerId, event)); + // We receive another receipt in the same event for another user + room.addReceipt(createReceipt(otherUserId, event)); + + // The event is not added in the room + // So the receipt is in a dangling state + expect(room.hasUserReadEvent(readerId, eventId)).toBe(false); + + // Add the event to the room + // The two receipts should be processed + room.addLiveEvents([event]); + + // Then the event is read + // We expect that the receipt of `otherUserId` didn't replace/erase the receipt of `readerId` + expect(room.hasUserReadEvent(readerId, eventId)).toBe(true); + }); + }); +}); + +function createFakeClient(): MatrixClient { + return { + getUserId: jest.fn(), + getEventMapper: jest.fn().mockReturnValue(jest.fn()), + isInitialSyncComplete: jest.fn().mockReturnValue(true), + supportsThreads: jest.fn().mockReturnValue(true), + fetchRoomEvent: jest.fn().mockResolvedValue({}), + paginateEventTimeline: jest.fn(), + canSupport: { get: jest.fn() }, + } as unknown as MatrixClient; +} + +const senderId = "sender:s.ss"; +const readerId = "reader:r.rr"; +const otherUserId = "other:o.oo"; + +function createRoom(): Room { + return new Room("!rid", createFakeClient(), "@u:s.nz", { timelineSupport: true }); +} + +let idCounter = 0; +function nextId(): string { + return "$" + (idCounter++).toString(10); +} + +/** + * Create an event and return it and its ID. + */ +function createEvent(): [MatrixEvent, string] { + return createEventSentBy(senderId); +} + +/** + * Create an event with the supplied sender and return it and its ID. + */ +function createEventSentBy(customSenderId: string): [MatrixEvent, string] { + const event = new MatrixEvent({ sender: customSenderId, event_id: nextId() }); + return [event, event.getId()!]; +} + +/** + * Create an event in the thread of the supplied root and return it and its ID. + */ +function createThreadedEvent(root: MatrixEvent): [MatrixEvent, string] { + const rootEventId = root.getId()!; + const event = new MatrixEvent({ + sender: senderId, + event_id: nextId(), + content: { + "m.relates_to": { + event_id: rootEventId, + rel_type: THREAD_RELATION_TYPE.name, + ["m.in_reply_to"]: { + event_id: rootEventId, + }, + }, + }, + }); + return [event, event.getId()!]; +} + +function createReceipt(userId: string, referencedEvent: MatrixEvent): MatrixEvent { + const content: ReceiptContent = { + [referencedEvent.getId()!]: { + "m.read": { + [userId]: { + ts: 123, + }, + }, + }, + }; + + return new MatrixEvent({ + type: "m.receipt", + content, + }); +} + +function createThreadedReceipt(userId: string, referencedEvent: MatrixEvent, threadId: string): MatrixEvent { + const content: ReceiptContent = { + [referencedEvent.getId()!]: { + "m.read": { + [userId]: { + ts: 123, + thread_id: threadId, + }, + }, + }, + }; + + return new MatrixEvent({ + type: "m.receipt", + content, + }); +} + +/** + * Create a timeline in the timeline set that is not the live timeline. + */ +function createOldTimeline(room: Room, events: MatrixEvent[]) { + const oldTimeline = room.getUnfilteredTimelineSet().addTimeline(); + room.getUnfilteredTimelineSet().addEventsToTimeline(events, true, oldTimeline); +} + +/** + * Perform the hacks required for this room to create a thread based on the root + * event supplied. + */ +function setupThread(room: Room, root: MatrixEvent) { + const thread = room.createThread(root.getId()!, root, [root], false); + thread.initialEventsFetched = true; +} diff --git a/spec/unit/models/thread.spec.ts b/spec/unit/models/thread.spec.ts index 05bd12200..38b9959ac 100644 --- a/spec/unit/models/thread.spec.ts +++ b/spec/unit/models/thread.spec.ts @@ -19,7 +19,7 @@ import { mocked } from "jest-mock"; import { MatrixClient, PendingEventOrdering } from "../../../src/client"; import { Room, RoomEvent } from "../../../src/models/room"; import { FeatureSupport, Thread, THREAD_RELATION_TYPE, ThreadEvent } from "../../../src/models/thread"; -import { makeThreadEvent, mkThread } from "../../test-utils/thread"; +import { makeThreadEvent, mkThread, populateThread } from "../../test-utils/thread"; import { TestClient } from "../../TestClient"; import { emitPromise, mkEdit, mkMessage, mkReaction, mock } from "../../test-utils/test-utils"; import { Direction, EventStatus, EventType, MatrixEvent } from "../../../src"; @@ -149,20 +149,38 @@ describe("Thread", () => { }); it("considers other events with no RR as unread", () => { - const { thread, events } = mkThread({ + // Given a long thread exists + const { thread, events } = populateThread({ room, client, - authorId: myUserId, - participantUserIds: [myUserId], + authorId: "@other:foo.com", + participantUserIds: ["@other:foo.com"], length: 25, ts: 190, }); - // Before alice's last unthreaded receipt - expect(thread.hasUserReadEvent("@alice:example.org", events.at(1)!.getId() ?? "")).toBeTruthy(); + const event1 = events.at(1)!; + const event2 = events.at(2)!; + const event24 = events.at(24)!; - // After alice's last unthreaded receipt - expect(thread.hasUserReadEvent("@alice:example.org", events.at(-1)!.getId() ?? "")).toBeFalsy(); + // And we have read the second message in it with an unthreaded receipt + const receipt = new MatrixEvent({ + type: "m.receipt", + room_id: room.roomId, + content: { + // unthreaded receipt for the second message in the thread + [event2.getId()!]: { + [ReceiptType.Read]: { + [myUserId]: { ts: 200 }, + }, + }, + }, + }); + room.addReceipt(receipt); + + // Then we have read the first message in the thread, and not the last + expect(thread.hasUserReadEvent(myUserId, event1.getId()!)).toBe(true); + expect(thread.hasUserReadEvent(myUserId, event24.getId()!)).toBe(false); }); it("considers event as read if there's a more recent unthreaded receipt", () => { @@ -481,13 +499,13 @@ describe("Thread", () => { // And a thread with an added event (with later timestamp) const userId = "user1"; - const { thread, message } = await createThreadAndEvent(client, 1, 100, userId); + const { thread, message2 } = await createThreadAnd2Events(client, 1, 100, 200, userId); // Then a receipt was added to the thread const receipt = thread.getReadReceiptForUserId(userId); expect(receipt).toBeTruthy(); - expect(receipt?.eventId).toEqual(message.getId()); - expect(receipt?.data.ts).toEqual(100); + expect(receipt?.eventId).toEqual(message2.getId()); + expect(receipt?.data.ts).toEqual(200); expect(receipt?.data.thread_id).toEqual(thread.id); // (And the receipt was synthetic) @@ -505,14 +523,14 @@ describe("Thread", () => { // And a thread with an added event with a lower timestamp than its other events const userId = "user1"; - const { thread } = await createThreadAndEvent(client, 200, 100, userId); + const { thread, message1 } = await createThreadAnd2Events(client, 300, 200, 100, userId); - // Then no receipt was added to the thread (the receipt is still - // for the thread root). This happens because since we have no + // Then the receipt is for the first message, because its + // timestamp is later. This happens because since we have no // recursive relations support, we know that sometimes events // appear out of order, so we have to check their timestamps as // a guess of the correct order. - expect(thread.getReadReceiptForUserId(userId)?.eventId).toEqual(thread.rootEvent?.getId()); + expect(thread.getReadReceiptForUserId(userId)?.eventId).toEqual(message1.getId()); }); }); @@ -530,11 +548,11 @@ describe("Thread", () => { // And a thread with an added event (with later timestamp) const userId = "user1"; - const { thread, message } = await createThreadAndEvent(client, 1, 100, userId); + const { thread, message2 } = await createThreadAnd2Events(client, 1, 100, 200, userId); // Then a receipt was added to the thread const receipt = thread.getReadReceiptForUserId(userId); - expect(receipt?.eventId).toEqual(message.getId()); + expect(receipt?.eventId).toEqual(message2.getId()); }); it("Creates a local echo receipt even for events BEFORE an existing receipt", async () => { @@ -550,22 +568,24 @@ describe("Thread", () => { // And a thread with an added event with a lower timestamp than its other events const userId = "user1"; - const { thread, message } = await createThreadAndEvent(client, 200, 100, userId); + const { thread, message2 } = await createThreadAnd2Events(client, 300, 200, 100, userId); - // Then a receipt was added to the thread, because relations - // recursion is available, so we trust the server to have - // provided us with events in the right order. + // Then a receipt was added for the last message, even though it + // has lower ts, because relations recursion is available, so we + // trust the server to have provided us with events in the right + // order. const receipt = thread.getReadReceiptForUserId(userId); - expect(receipt?.eventId).toEqual(message.getId()); + expect(receipt?.eventId).toEqual(message2.getId()); }); }); - async function createThreadAndEvent( + async function createThreadAnd2Events( client: MatrixClient, rootTs: number, - eventTs: number, + message1Ts: number, + message2Ts: number, userId: string, - ): Promise<{ thread: Thread; message: MatrixEvent }> { + ): Promise<{ thread: Thread; message1: MatrixEvent; message2: MatrixEvent }> { const room = new Room("room1", client, userId); // Given a thread @@ -576,24 +596,41 @@ describe("Thread", () => { participantUserIds: [], ts: rootTs, }); - // Sanity: the current receipt is for the thread root - expect(thread.getReadReceiptForUserId(userId)?.eventId).toEqual(thread.rootEvent?.getId()); + // Sanity: there is no read receipt on the thread yet because the + // thread events don't get properly added to the room by mkThread. + expect(thread.getReadReceiptForUserId(userId)).toBeNull(); const awaitTimelineEvent = new Promise((res) => thread.on(RoomEvent.Timeline, () => res())); - // When we add a message that is before the latest receipt - const message = makeThreadEvent({ + // Add a message with ts message1Ts + const message1 = makeThreadEvent({ event: true, rootEventId: thread.id, replyToEventId: thread.id, user: userId, room: room.roomId, - ts: eventTs, + ts: message1Ts, }); - await thread.addEvent(message, false, true); + await thread.addEvent(message1, false, true); await awaitTimelineEvent; - return { thread, message }; + // Sanity: the thread now has a properly-added event, so this event + // has a synthetic receipt. + expect(thread.getReadReceiptForUserId(userId)?.eventId).toEqual(message1.getId()); + + // Add a message with ts message2Ts + const message2 = makeThreadEvent({ + event: true, + rootEventId: thread.id, + replyToEventId: thread.id, + user: userId, + room: room.roomId, + ts: message2Ts, + }); + await thread.addEvent(message2, false, true); + await awaitTimelineEvent; + + return { thread, message1, message2 }; } function createClientWithEventMapper(canSupport: Map = new Map()): MatrixClient { diff --git a/spec/unit/read-receipt.spec.ts b/spec/unit/read-receipt.spec.ts index ea27d5152..1fe7d4dc3 100644 --- a/spec/unit/read-receipt.spec.ts +++ b/spec/unit/read-receipt.spec.ts @@ -43,8 +43,10 @@ const THREAD_ID = "$thread_event_id"; const ROOM_ID = "!123:matrix.org"; describe("Read receipt", () => { + let threadRoot: MatrixEvent; let threadEvent: MatrixEvent; let roomEvent: MatrixEvent; + let editOfThreadRoot: MatrixEvent; beforeEach(() => { httpBackend = new MockHttpBackend(); @@ -57,6 +59,15 @@ describe("Read receipt", () => { client.isGuest = () => false; client.supportsThreads = () => true; + threadRoot = utils.mkEvent({ + event: true, + type: EventType.RoomMessage, + user: "@bob:matrix.org", + room: ROOM_ID, + content: { body: "This is the thread root" }, + }); + threadRoot.event.event_id = THREAD_ID; + threadEvent = utils.mkEvent({ event: true, type: EventType.RoomMessage, @@ -82,6 +93,9 @@ describe("Read receipt", () => { body: "Hello from a room", }, }); + + editOfThreadRoot = utils.mkEdit(threadRoot, client, "@bob:matrix.org", ROOM_ID); + editOfThreadRoot.setThreadId(THREAD_ID); }); describe("sendReceipt", () => { @@ -208,6 +222,7 @@ describe("Read receipt", () => { it.each([ { getEvent: () => roomEvent, destinationId: MAIN_ROOM_TIMELINE }, { getEvent: () => threadEvent, destinationId: THREAD_ID }, + { getEvent: () => editOfThreadRoot, destinationId: MAIN_ROOM_TIMELINE }, ])("adds the receipt to $destinationId", ({ getEvent, destinationId }) => { const event = getEvent(); const userId = "@bob:example.org"; diff --git a/spec/unit/room.spec.ts b/spec/unit/room.spec.ts index a8634100f..93a096f17 100644 --- a/spec/unit/room.spec.ts +++ b/spec/unit/room.spec.ts @@ -1743,13 +1743,70 @@ describe("Room", function () { }); describe("hasUserReadUpTo", function () { - it("should acknowledge if an event has been read", function () { + it("returns true if there is a receipt for this event (main timeline)", function () { const ts = 13787898424; + room.addLiveEvents([eventToAck]); room.addReceipt(mkReceipt(roomId, [mkRecord(eventToAck.getId()!, "m.read", userB, ts)])); - room.findEventById = jest.fn().mockReturnValue({} as MatrixEvent); + room.findEventById = jest.fn().mockReturnValue({ getThread: jest.fn() } as unknown as MatrixEvent); expect(room.hasUserReadEvent(userB, eventToAck.getId()!)).toEqual(true); }); - it("return false for an unknown event", function () { + + it("returns true if there is a receipt for a later event (main timeline)", async function () { + // Given some events exist in the room + const events: MatrixEvent[] = [ + utils.mkMessage({ + room: roomId, + user: userA, + msg: "1111", + event: true, + }), + utils.mkMessage({ + room: roomId, + user: userA, + msg: "2222", + event: true, + }), + utils.mkMessage({ + room: roomId, + user: userA, + msg: "3333", + event: true, + }), + ]; + await room.addLiveEvents(events); + + // When I add a receipt for the latest one + room.addReceipt(mkReceipt(roomId, [mkRecord(events[2].getId()!, "m.read", userB, 102)])); + + // Then the older ones are read too + expect(room.hasUserReadEvent(userB, events[0].getId()!)).toEqual(true); + expect(room.hasUserReadEvent(userB, events[1].getId()!)).toEqual(true); + }); + + describe("threads enabled", () => { + beforeEach(() => { + jest.spyOn(room.client, "supportsThreads").mockReturnValue(true); + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + it("returns true if there is an unthreaded receipt for a later event in a thread", async () => { + // Given a thread exists in the room + const { thread, events } = mkThread({ room, length: 3 }); + thread.initialEventsFetched = true; + await room.addLiveEvents(events); + + // When I add an unthreaded receipt for the latest thread message + room.addReceipt(mkReceipt(roomId, [mkRecord(events[2].getId()!, "m.read", userB, 102)])); + + // Then the main timeline message is read + expect(room.hasUserReadEvent(userB, events[0].getId()!)).toEqual(true); + }); + }); + + it("returns false for an unknown event", function () { expect(room.hasUserReadEvent(userB, "unknown_event")).toEqual(false); }); }); diff --git a/src/client.ts b/src/client.ts index a294dcbd8..5d436a1fc 100644 --- a/src/client.ts +++ b/src/client.ts @@ -5168,7 +5168,7 @@ export class MatrixClient extends TypedEventEmitter right, 0 if left == right, null if + * we can't tell (because we can't find the events). + */ +export function compareEventOrdering(room: Room, leftEventId: string, rightEventId: string): number | null { + const leftEvent = room.findEventById(leftEventId); + const rightEvent = room.findEventById(rightEventId); + + if (!leftEvent || !rightEvent) { + // Without the events themselves, we can't find their thread or + // timeline, or guess based on timestamp, so we just don't know. + return null; + } + + // Check whether the events are in the main timeline + const isLeftEventInMainTimeline = inMainTimelineForReceipt(leftEvent); + const isRightEventInMainTimeline = inMainTimelineForReceipt(rightEvent); + + if (isLeftEventInMainTimeline && isRightEventInMainTimeline) { + return compareEventsInMainTimeline(room, leftEventId, rightEventId, leftEvent, rightEvent); + } else { + // At least one event is not in the timeline, so we can't use the room's + // unfiltered timeline set. + return compareEventsInThreads(leftEventId, rightEventId, leftEvent, rightEvent); + } +} + +function compareEventsInMainTimeline( + room: Room, + leftEventId: string, + rightEventId: string, + leftEvent: MatrixEvent, + rightEvent: MatrixEvent, +): number | null { + // Get the timeline set that contains all the events. + const timelineSet = room.getUnfilteredTimelineSet(); + + // If they are in the same timeline, compareEventOrdering does what we need + const compareSameTimeline = timelineSet.compareEventOrdering(leftEventId, rightEventId); + if (compareSameTimeline !== null) { + return compareSameTimeline; + } + + // Find which timeline each event is in. Refuse to provide an ordering if we + // can't find either of the events. + + const leftTimeline = timelineSet.getTimelineForEvent(leftEventId); + if (leftTimeline === timelineSet.getLiveTimeline()) { + // The left event is part of the live timeline, so it must be after the + // right event (since they are not in the same timeline or we would have + // returned after compareEventOrdering. + return 1; + } + + const rightTimeline = timelineSet.getTimelineForEvent(rightEventId); + if (rightTimeline === timelineSet.getLiveTimeline()) { + // The right event is part of the live timeline, so it must be after the + // left event. + return -1; + } + + // They are in older timeline sets (because they were fetched by paging up). + return guessOrderBasedOnTimestamp(leftEvent, rightEvent); +} + +function compareEventsInThreads( + leftEventId: string, + rightEventId: string, + leftEvent: MatrixEvent, + rightEvent: MatrixEvent, +): number | null { + const leftEventThreadId = threadIdForReceipt(leftEvent); + const rightEventThreadId = threadIdForReceipt(rightEvent); + + const leftThread = leftEvent.getThread(); + + if (leftThread && leftEventThreadId === rightEventThreadId) { + // They are in the same thread, so we can ask the thread's timeline to + // figure it out for us + return leftThread.timelineSet.compareEventOrdering(leftEventId, rightEventId); + } else { + return guessOrderBasedOnTimestamp(leftEvent, rightEvent); + } +} + +/** + * Guess the order of events based on server timestamp. This is not good, but + * difficult to avoid without MSC4033. + * + * See https://github.com/matrix-org/matrix-js-sdk/issues/3325 + */ +function guessOrderBasedOnTimestamp(leftEvent: MatrixEvent, rightEvent: MatrixEvent): number { + const leftTs = leftEvent.getTs(); + const rightTs = rightEvent.getTs(); + if (leftTs < rightTs) { + return -1; + } else if (leftTs > rightTs) { + return 1; + } else { + return 0; + } +} diff --git a/src/models/event-timeline-set.ts b/src/models/event-timeline-set.ts index 0427967fd..3fce566e7 100644 --- a/src/models/event-timeline-set.ts +++ b/src/models/event-timeline-set.ts @@ -899,11 +899,10 @@ export class EventTimelineSet extends TypedEventEmitter 0) { + return 1; + } else { + return 0; + } } // the events are in different timelines. Iterate through the diff --git a/src/models/read-receipt.ts b/src/models/read-receipt.ts index a82770814..08f0f4909 100644 --- a/src/models/read-receipt.ts +++ b/src/models/read-receipt.ts @@ -27,15 +27,29 @@ import { EventTimelineSet } from "./event-timeline-set"; import { MapWithDefault } from "../utils"; import { NotificationCountType } from "./room"; import { logger } from "../logger"; +import { inMainTimelineForReceipt, threadIdForReceipt } from "../client"; -export function synthesizeReceipt(userId: string, event: MatrixEvent, receiptType: ReceiptType): MatrixEvent { +/** + * Create a synthetic receipt for the given event + * @param userId - The user ID if the receipt sender + * @param event - The event that is to be acknowledged + * @param receiptType - The type of receipt + * @param unthreaded - the receipt is unthreaded + * @returns a new event with the synthetic receipt in it + */ +export function synthesizeReceipt( + userId: string, + event: MatrixEvent, + receiptType: ReceiptType, + unthreaded = false, +): MatrixEvent { return new MatrixEvent({ content: { [event.getId()!]: { [receiptType]: { [userId]: { ts: event.getTs(), - thread_id: event.threadRootId ?? MAIN_ROOM_TIMELINE, + ...(!unthreaded && { thread_id: threadIdForReceipt(event) }), }, }, }, @@ -160,11 +174,8 @@ export abstract class ReadReceipt< // The receipt is for the main timeline: we check that the event is // in the main timeline. - // There are two ways to know an event is in the main timeline: - // either it has no threadRootId, or it is a thread root. - // (Note: it's a little odd because the thread root is in the main - // timeline, but it still has a threadRootId.) - const eventIsInMainTimeline = !event.threadRootId || event.isThreadRoot; + // Check if the event is in the main timeline + const eventIsInMainTimeline = inMainTimelineForReceipt(event); if (eventIsInMainTimeline) { // The receipt is for the main timeline, and so is the event, so @@ -367,9 +378,10 @@ export abstract class ReadReceipt< * @param userId - The user ID if the receipt sender * @param e - The event that is to be acknowledged * @param receiptType - The type of receipt + * @param unthreaded - the receipt is unthreaded */ - public addLocalEchoReceipt(userId: string, e: MatrixEvent, receiptType: ReceiptType): void { - this.addReceipt(synthesizeReceipt(userId, e, receiptType), true); + public addLocalEchoReceipt(userId: string, e: MatrixEvent, receiptType: ReceiptType, unthreaded = false): void { + this.addReceipt(synthesizeReceipt(userId, e, receiptType, unthreaded), true); } /** @@ -395,33 +407,7 @@ export abstract class ReadReceipt< * @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 { - const readUpToId = this.getEventReadUpTo(userId, false); - if (readUpToId === eventId) return true; - - if ( - this.timeline?.length && - this.timeline[this.timeline.length - 1].getSender() && - this.timeline[this.timeline.length - 1].getSender() === userId - ) { - // It doesn't matter where the event is in the timeline, the user has read - // it because they've sent the latest event. - return true; - } - - for (let i = this.timeline?.length - 1; i >= 0; --i) { - const ev = this.timeline[i]; - - // If we encounter the target event first, the user hasn't read it - // however if we encounter the readUpToId first then the user has read - // it. These rules apply because we're iterating bottom-up. - if (ev.getId() === eventId) return false; - if (ev.getId() === readUpToId) return true; - } - - // We don't know if the user has read it, so assume not. - return false; - } + public abstract hasUserReadEvent(userId: string, eventId: string): boolean; /** * Returns the most recent unthreaded receipt for a given user @@ -429,6 +415,8 @@ export abstract class ReadReceipt< * @returns an unthreaded Receipt. Can be undefined if receipts have been disabled * or a user chooses to use private read receipts (or we have simply not received * a receipt from this user yet). + * + * @deprecated use `hasUserReadEvent` or `getEventReadUpTo` instead */ public abstract getLastUnthreadedReceiptFor(userId: string): Receipt | undefined; } diff --git a/src/models/room-receipts.ts b/src/models/room-receipts.ts new file mode 100644 index 000000000..6c0f7484e --- /dev/null +++ b/src/models/room-receipts.ts @@ -0,0 +1,429 @@ +/* +Copyright 2023 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 { MAIN_ROOM_TIMELINE, Receipt, ReceiptContent } from "../@types/read_receipts"; +import { threadIdForReceipt } from "../client"; +import { Room, RoomEvent } from "./room"; +import { MatrixEvent } from "./event"; + +/** + * The latest receipts we have for a room. + */ +export class RoomReceipts { + private room: Room; + private threadedReceipts: ThreadedReceipts; + private unthreadedReceipts: ReceiptsByUser; + private danglingReceipts: DanglingReceipts; + + public constructor(room: Room) { + this.room = room; + this.threadedReceipts = new ThreadedReceipts(room); + this.unthreadedReceipts = new ReceiptsByUser(room); + this.danglingReceipts = new DanglingReceipts(); + // We listen for timeline events so we can process dangling receipts + room.on(RoomEvent.Timeline, this.onTimelineEvent); + } + + /** + * Remember the receipt information supplied. For each receipt: + * + * If we don't have the event for this receipt, store it as "dangling" so we + * can process it later. + * + * Otherwise store it per-user in either the threaded store for its + * thread_id, or the unthreaded store if there is no thread_id. + * + * Ignores any receipt that is before an existing receipt for the same user + * (in the same thread, if applicable). "Before" is defined by the + * unfilteredTimelineSet of the room. + */ + public add(receiptContent: ReceiptContent, synthetic: boolean): void { + /* + Transform this structure: + { + "$EVENTID": { + "m.read|m.read.private": { + "@user:example.org": { + "ts": 1661, + "thread_id": "main|$THREAD_ROOT_ID" // or missing/undefined for an unthreaded receipt + } + } + }, + ... + } + into maps of: + threaded :: threadid :: userId :: ReceiptInfo + unthreaded :: userId :: ReceiptInfo + dangling :: eventId :: DanglingReceipt + */ + for (const [eventId, eventReceipt] of Object.entries(receiptContent)) { + for (const [receiptType, receiptsByUser] of Object.entries(eventReceipt)) { + for (const [userId, receipt] of Object.entries(receiptsByUser)) { + const referencedEvent = this.room.findEventById(eventId); + if (!referencedEvent) { + this.danglingReceipts.add( + new DanglingReceipt(eventId, receiptType, userId, receipt, synthetic), + ); + } else if (receipt.thread_id) { + this.threadedReceipts.set( + receipt.thread_id, + eventId, + receiptType, + userId, + receipt.ts, + synthetic, + ); + } else { + this.unthreadedReceipts.set(eventId, receiptType, userId, receipt.ts, synthetic); + } + } + } + } + } + + /** + * Look for dangling receipts for the given event ID, + * and add them to the thread of unthread receipts if found. + * @param eventId - the event ID to look for + */ + private onTimelineEvent = (event: MatrixEvent): void => { + const eventId = event.getId(); + if (!eventId) return; + + const danglingReceipts = this.danglingReceipts.remove(eventId); + + danglingReceipts?.forEach((danglingReceipt) => { + // The receipt is a thread receipt + if (danglingReceipt.receipt.thread_id) { + this.threadedReceipts.set( + danglingReceipt.receipt.thread_id, + danglingReceipt.eventId, + danglingReceipt.receiptType, + danglingReceipt.userId, + danglingReceipt.receipt.ts, + danglingReceipt.synthetic, + ); + } else { + this.unthreadedReceipts.set( + eventId, + danglingReceipt.receiptType, + danglingReceipt.userId, + danglingReceipt.receipt.ts, + danglingReceipt.synthetic, + ); + } + }); + }; + + public hasUserReadEvent(userId: string, eventId: string): boolean { + const unthreaded = this.unthreadedReceipts.get(userId); + if (unthreaded) { + if (isAfterOrSame(unthreaded.eventId, eventId, this.room)) { + // The unthreaded receipt is after this event, so we have read it. + return true; + } + } + + const event = this.room.findEventById(eventId); + if (!event) { + // We don't know whether the user has read it - default to caution and say no. + return false; + } + + const threadId = threadIdForReceipt(event); + const threaded = this.threadedReceipts.get(threadId, userId); + if (threaded) { + if (isAfterOrSame(threaded.eventId, eventId, this.room)) { + // The threaded receipt is after this event, so we have read it. + return true; + } + } + + // TODO: what if they sent the second-last event in the thread? + if (this.userSentLatestEventInThread(threadId, userId)) { + // The user sent the latest message in this event's thread, so we + // consider everything in the thread to be read. + // + // Note: maybe we don't need this because synthetic receipts should + // do this job for us? + return true; + } + + // Neither of the receipts were after the event, so it's unread. + return false; + } + + /** + * @returns true if the thread with this ID can be found, and the supplied + * user sent the latest message in it. + */ + private userSentLatestEventInThread(threadId: string, userId: String): boolean { + const timeline = + threadId === MAIN_ROOM_TIMELINE + ? this.room.getLiveTimeline().getEvents() + : this.room.getThread(threadId)?.timeline; + + return !!(timeline && timeline.length > 0 && timeline[timeline.length - 1].getSender() === userId); + } +} + +// --- implementation details --- + +/** + * The information "inside" a receipt once it has been stored inside + * RoomReceipts - what eventId it refers to, its type, and its ts. + * + * Does not contain userId or threadId since these are stored as keys of the + * maps in RoomReceipts. + */ +class ReceiptInfo { + public constructor(public eventId: string, public receiptType: string, public ts: number) {} +} + +/** + * Everything we know about a receipt that is "dangling" because we can't find + * the event to which it refers. + */ +class DanglingReceipt { + public constructor( + public eventId: string, + public receiptType: string, + public userId: string, + public receipt: Receipt, + public synthetic: boolean, + ) {} +} + +class UserReceipts { + private room: Room; + + /** + * The real receipt for this user. + */ + private real: ReceiptInfo | undefined; + + /** + * The synthetic receipt for this user. If this is defined, it is later than real. + */ + private synthetic: ReceiptInfo | undefined; + + public constructor(room: Room) { + this.room = room; + this.real = undefined; + this.synthetic = undefined; + } + + public set(synthetic: boolean, receiptInfo: ReceiptInfo): void { + if (synthetic) { + this.synthetic = receiptInfo; + } else { + this.real = receiptInfo; + } + + // Preserve the invariant: synthetic is only defined if it's later than real + if (this.synthetic && this.real) { + if (isAfterOrSame(this.real.eventId, this.synthetic.eventId, this.room)) { + this.synthetic = undefined; + } + } + } + + /** + * Return the latest receipt we have - synthetic if we have one (and it's + * later), otherwise real. + */ + public get(): ReceiptInfo | undefined { + // Relies on the invariant that synthetic is only defined if it's later than real. + return this.synthetic ?? this.real; + } + + /** + * Return the latest receipt we have of the specified type (synthetic or not). + */ + public getByType(synthetic: boolean): ReceiptInfo | undefined { + return synthetic ? this.synthetic : this.real; + } +} + +/** + * The latest receipt info we have, either for a single thread, or all the + * unthreaded receipts for a room. + * + * userId: ReceiptInfo + */ +class ReceiptsByUser { + private room: Room; + + /** map of userId: UserReceipts */ + private data: Map; + + public constructor(room: Room) { + this.room = room; + this.data = new Map(); + } + + /** + * Add the supplied receipt to our structure, if it is not earlier than the + * one we already hold for this user. + */ + public set(eventId: string, receiptType: string, userId: string, ts: number, synthetic: boolean): void { + const userReceipts = getOrCreate(this.data, userId, () => new UserReceipts(this.room)); + + const existingReceipt = userReceipts.getByType(synthetic); + if (existingReceipt && isAfter(existingReceipt.eventId, eventId, this.room)) { + // The new receipt is before the existing one - don't store it. + return; + } + + // Possibilities: + // + // 1. there was no existing receipt, or + // 2. the existing receipt was before this one, or + // 3. we were unable to compare the receipts. + // + // In the case of 3 it's difficult to decide what to do, so the + // most-recently-received receipt wins. + // + // Case 3 can only happen if the events for these receipts have + // disappeared, which is quite unlikely since the new one has just been + // checked, and the old one was checked before it was inserted here. + // + // We go ahead and store this receipt (replacing the other if it exists) + userReceipts.set(synthetic, new ReceiptInfo(eventId, receiptType, ts)); + } + + /** + * Find the latest receipt we have for this user. (Note - there is only one + * receipt per user, because we are already inside a specific thread or + * unthreaded list.) + * + * If there is a later synthetic receipt for this user, return that. + * Otherwise, return the real receipt. + * + * @returns the found receipt info, or undefined if we have no receipt for this user. + */ + public get(userId: string): ReceiptInfo | undefined { + return this.data.get(userId)?.get(); + } +} + +/** + * The latest threaded receipts we have for a room. + */ +class ThreadedReceipts { + private room: Room; + + /** map of threadId: ReceiptsByUser */ + private data: Map; + + public constructor(room: Room) { + this.room = room; + this.data = new Map(); + } + + /** + * Add the supplied receipt to our structure, if it is not earlier than one + * we already hold for this user in this thread. + */ + public set( + threadId: string, + eventId: string, + receiptType: string, + userId: string, + ts: number, + synthetic: boolean, + ): void { + const receiptsByUser = getOrCreate(this.data, threadId, () => new ReceiptsByUser(this.room)); + receiptsByUser.set(eventId, receiptType, userId, ts, synthetic); + } + + /** + * Find the latest threaded receipt for the supplied user in the supplied thread. + * + * @returns the found receipt info or undefined if we don't have one. + */ + public get(threadId: string, userId: string): ReceiptInfo | undefined { + return this.data.get(threadId)?.get(userId); + } +} + +/** + * All the receipts that we have received but can't process because we can't + * find the event they refer to. + * + * We hold on to them so we can process them if their event arrives later. + */ +class DanglingReceipts { + /** + * eventId: DanglingReceipt[] + */ + private data = new Map>(); + + /** + * Remember the supplied dangling receipt. + */ + public add(danglingReceipt: DanglingReceipt): void { + const danglingReceipts = getOrCreate(this.data, danglingReceipt.eventId, () => []); + danglingReceipts.push(danglingReceipt); + } + + /** + * Remove and return the dangling receipts for the given event ID. + * @param eventId - the event ID to look for + * @returns the found dangling receipts, or undefined if we don't have one. + */ + public remove(eventId: string): Array | undefined { + const danglingReceipts = this.data.get(eventId); + this.data.delete(eventId); + return danglingReceipts; + } +} + +function getOrCreate(m: Map, key: K, createFn: () => V): V { + const found = m.get(key); + if (found) { + return found; + } else { + const created = createFn(); + m.set(key, created); + return created; + } +} + +/** + * Is left after right (or the same)? + * + * Only returns true if both events can be found, and left is after or the same + * as right. + * + * @returns left \>= right + */ +function isAfterOrSame(leftEventId: string, rightEventId: string, room: Room): boolean { + const comparison = room.compareEventOrdering(leftEventId, rightEventId); + return comparison !== null && comparison >= 0; +} + +/** + * Is left strictly after right? + * + * Only returns true if both events can be found, and left is strictly after right. + * + * @returns left \> right + */ +function isAfter(leftEventId: string, rightEventId: string, room: Room): boolean { + const comparison = room.compareEventOrdering(leftEventId, rightEventId); + return comparison !== null && comparison > 0; +} diff --git a/src/models/room.ts b/src/models/room.ts index 17b211efc..48fabcfce 100644 --- a/src/models/room.ts +++ b/src/models/room.ts @@ -66,6 +66,8 @@ import { IStateEventWithRoomId } from "../@types/search"; import { RelationsContainer } from "./relations-container"; import { ReadReceipt, synthesizeReceipt } from "./read-receipt"; import { isPollEvent, Poll, PollEvent } from "./poll"; +import { RoomReceipts } from "./room-receipts"; +import { compareEventOrdering } from "./compare-event-ordering"; // These constants are used as sane defaults when the homeserver doesn't support // the m.room_versions capability. In practice, KNOWN_SAFE_ROOM_VERSION should be @@ -432,6 +434,12 @@ export class Room extends ReadReceipt { */ private visibilityEvents = new Map(); + /** + * The latest receipts (synthetic and real) for each user in each thread + * (and unthreaded). + */ + private roomReceipts = new RoomReceipts(this); + /** * Construct a new Room. * @@ -2935,6 +2943,10 @@ export class Room extends ReadReceipt { */ public addReceipt(event: MatrixEvent, synthetic = false): void { const content = event.getContent(); + + this.roomReceipts.add(content, synthetic); + + // TODO: delete the following code when it has been replaced by RoomReceipts Object.keys(content).forEach((eventId: string) => { Object.keys(content[eventId]).forEach((receiptType: ReceiptType | string) => { Object.keys(content[eventId][receiptType]).forEach((userId: string) => { @@ -2996,6 +3008,7 @@ export class Room extends ReadReceipt { }); }); }); + // End of code to delete when replaced by RoomReceipts // send events after we've regenerated the structure & cache, otherwise things that // listened for the event would read stale data. @@ -3582,6 +3595,19 @@ export class Room extends ReadReceipt { return this.oldestThreadedReceiptTs; } + /** + * 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 room 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 { + return this.roomReceipts.hasUserReadEvent(userId, eventId); + } + /** * Returns the most recent unthreaded receipt for a given user * @param userId - the MxID of the User @@ -3615,6 +3641,30 @@ export class Room extends ReadReceipt { thread.fixupNotifications(userId); } } + + /** + * Determine the order of two events in this room. + * + * In principle this should use the same order as the server, but in practice + * this is difficult for events that were not received over the Sync API. See + * MSC4033 for details. + * + * This implementation leans on the order of events within their timelines, and + * falls back to comparing event timestamps when they are in different + * timelines. + * + * See https://github.com/matrix-org/matrix-js-sdk/issues/3325 for where we are + * tracking the work to fix this. + * + * @param leftEventId - the id of the first event + * @param rightEventId - the id of the second event + + * @returns -1 if left \< right, 1 if left \> right, 0 if left == right, null if + * we can't tell (because we can't find the events). + */ + public compareEventOrdering(leftEventId: string, rightEventId: string): number | null { + return compareEventOrdering(this, leftEventId, rightEventId); + } } // a map from current event status to a list of allowed next statuses diff --git a/src/models/thread.ts b/src/models/thread.ts index 244369557..e4cfdaadb 100644 --- a/src/models/thread.ts +++ b/src/models/thread.ts @@ -748,6 +748,27 @@ export class Thread extends ReadReceipt