From 737dfadb3a18412ba6d7504f9f6c106b31a6275d Mon Sep 17 00:00:00 2001 From: Germain Date: Tue, 15 Nov 2022 19:42:21 +0000 Subject: [PATCH] Allow read receipt accumulation per room Following MSC3771, read receipts are not unique per room per user id. It is now possible for a single user id to have multiple read receipts across threads and the main room timeline. The sync accumulator was not taking this into consideration and was always persisting the last read receipt it had received --- spec/unit/read-receipt.spec.ts | 3 +- spec/unit/room.spec.ts | 3 +- spec/unit/sync-accumulator.spec.ts | 57 ++++++++++++++++++++++++++++ src/@types/read_receipts.ts | 35 +++++++++++++++++ src/client.ts | 3 +- src/models/read-receipt.ts | 44 +++++---------------- src/models/room.ts | 5 +-- src/sync-accumulator.ts | 61 ++++++++++++++++++++++-------- 8 files changed, 151 insertions(+), 60 deletions(-) diff --git a/spec/unit/read-receipt.spec.ts b/spec/unit/read-receipt.spec.ts index 2e3870a388b..2a3fbd87bcc 100644 --- a/spec/unit/read-receipt.spec.ts +++ b/spec/unit/read-receipt.spec.ts @@ -16,11 +16,10 @@ limitations under the License. import MockHttpBackend from 'matrix-mock-request'; -import { ReceiptType } from '../../src/@types/read_receipts'; +import { MAIN_ROOM_TIMELINE, ReceiptType } from '../../src/@types/read_receipts'; import { MatrixClient } from "../../src/client"; import { Feature, ServerSupport } from '../../src/feature'; import { EventType } from '../../src/matrix'; -import { MAIN_ROOM_TIMELINE } from '../../src/models/read-receipt'; import { encodeUri } from '../../src/utils'; import * as utils from "../test-utils/test-utils"; diff --git a/spec/unit/room.spec.ts b/spec/unit/room.spec.ts index d9a30e10cb7..f6199570325 100644 --- a/spec/unit/room.spec.ts +++ b/spec/unit/room.spec.ts @@ -38,9 +38,8 @@ import { RoomState } from "../../src/models/room-state"; import { UNSTABLE_ELEMENT_FUNCTIONAL_USERS } from "../../src/@types/event"; import { TestClient } from "../TestClient"; import { emitPromise } from "../test-utils/test-utils"; -import { ReceiptType } from "../../src/@types/read_receipts"; +import { ReceiptType, WrappedReceipt } from "../../src/@types/read_receipts"; import { FeatureSupport, Thread, ThreadEvent, THREAD_RELATION_TYPE } from "../../src/models/thread"; -import { WrappedReceipt } from "../../src/models/read-receipt"; import { Crypto } from "../../src/crypto"; describe("Room", function() { diff --git a/spec/unit/sync-accumulator.spec.ts b/spec/unit/sync-accumulator.spec.ts index 5618dbe2239..2e1ec58b60c 100644 --- a/spec/unit/sync-accumulator.spec.ts +++ b/spec/unit/sync-accumulator.spec.ts @@ -364,6 +364,63 @@ describe("SyncAccumulator", function() { }); }); + it("should accumulate threaded read receipts", () => { + const receipt1 = { + type: "m.receipt", + room_id: "!foo:bar", + content: { + "$event1:localhost": { + [ReceiptType.Read]: { + "@alice:localhost": { ts: 1, thread_id: "main" }, + }, + }, + }, + }; + const receipt2 = { + type: "m.receipt", + room_id: "!foo:bar", + content: { + "$event2:localhost": { + [ReceiptType.Read]: { + "@alice:localhost": { ts: 2, thread_id: "$123" }, // does not clobbers event1 receipt + }, + }, + }, + }; + sa.accumulate(syncSkeleton({ + ephemeral: { + events: [receipt1], + }, + })); + sa.accumulate(syncSkeleton({ + ephemeral: { + events: [receipt2], + }, + })); + + expect( + sa.getJSON().roomsData.join["!foo:bar"].ephemeral.events.length, + ).toEqual(1); + expect( + sa.getJSON().roomsData.join["!foo:bar"].ephemeral.events[0], + ).toEqual({ + type: "m.receipt", + room_id: "!foo:bar", + content: { + "$event1:localhost": { + [ReceiptType.Read]: { + "@alice:localhost": { ts: 1, thread_id: "main" }, + }, + }, + "$event2:localhost": { + [ReceiptType.Read]: { + "@alice:localhost": { ts: 2, thread_id: "$123" }, + }, + }, + }, + }); + }); + describe("summary field", function() { function createSyncResponseWithSummary(summary) { return { diff --git a/src/@types/read_receipts.ts b/src/@types/read_receipts.ts index 4e90ac2ea94..689313672a3 100644 --- a/src/@types/read_receipts.ts +++ b/src/@types/read_receipts.ts @@ -19,3 +19,38 @@ export enum ReceiptType { FullyRead = "m.fully_read", ReadPrivate = "m.read.private", } + +export const MAIN_ROOM_TIMELINE = "main"; + +export interface Receipt { + ts: number; + thread_id?: string; +} + +export interface WrappedReceipt { + eventId: string; + data: Receipt; +} + +export interface CachedReceipt { + type: ReceiptType; + userId: string; + data: Receipt; +} + +export type ReceiptCache = {[eventId: string]: CachedReceipt[]}; + +export interface ReceiptContent { + [eventId: string]: { + [key in ReceiptType]: { + [userId: string]: Receipt; + }; + }; +} + +// We will only hold a synthetic receipt if we do not have a real receipt or the synthetic is newer. +export type Receipts = { + [receiptType: string]: { + [userId: string]: [WrappedReceipt | null, WrappedReceipt | null]; // Pair (both nullable) + }; +}; diff --git a/src/client.ts b/src/client.ts index 64e9f6af79f..a3b44dfa0a2 100644 --- a/src/client.ts +++ b/src/client.ts @@ -195,7 +195,7 @@ import { MediaHandler } from "./webrtc/mediaHandler"; import { GroupCallEventHandler } from "./webrtc/groupCallEventHandler"; import { LoginTokenPostResponse, ILoginFlowsResponse, IRefreshTokenResponse, SSOAction } from "./@types/auth"; import { TypedEventEmitter } from "./models/typed-event-emitter"; -import { ReceiptType } from "./@types/read_receipts"; +import { MAIN_ROOM_TIMELINE, ReceiptType } from "./@types/read_receipts"; import { MSC3575SlidingSyncRequest, MSC3575SlidingSyncResponse, SlidingSync } from "./sliding-sync"; import { SlidingSyncSdk } from "./sliding-sync-sdk"; import { @@ -210,7 +210,6 @@ import { MBeaconInfoEventContent, M_BEACON_INFO } from "./@types/beacon"; import { UnstableValue } from "./NamespacedValue"; import { ToDeviceMessageQueue } from "./ToDeviceMessageQueue"; import { ToDeviceBatch } from "./models/ToDeviceMessage"; -import { MAIN_ROOM_TIMELINE } from "./models/read-receipt"; import { IgnoredInvites } from "./models/invites-ignorer"; import { UIARequest, UIAResponse } from "./@types/uia"; import { LocalNotificationSettings } from "./@types/local_notifications"; diff --git a/src/models/read-receipt.ts b/src/models/read-receipt.ts index b28976c5c6b..3d78991478f 100644 --- a/src/models/read-receipt.ts +++ b/src/models/read-receipt.ts @@ -11,15 +11,21 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { ReceiptType } from "../@types/read_receipts"; +import { + CachedReceipt, + MAIN_ROOM_TIMELINE, + Receipt, + ReceiptCache, + Receipts, + ReceiptType, + WrappedReceipt, +} from "../@types/read_receipts"; import { ListenerMap, TypedEventEmitter } from "./typed-event-emitter"; import * as utils from "../utils"; import { MatrixEvent } from "./event"; import { EventType } from "../@types/event"; import { EventTimelineSet } from "./event-timeline-set"; -export const MAIN_ROOM_TIMELINE = "main"; - export function synthesizeReceipt(userId: string, event: MatrixEvent, receiptType: ReceiptType): MatrixEvent { return new MatrixEvent({ content: { @@ -37,40 +43,8 @@ export function synthesizeReceipt(userId: string, event: MatrixEvent, receiptTyp }); } -export interface Receipt { - ts: number; - thread_id?: string; -} - -export interface WrappedReceipt { - eventId: string; - data: Receipt; -} - -interface CachedReceipt { - type: ReceiptType; - userId: string; - data: Receipt; -} - -type ReceiptCache = {[eventId: string]: CachedReceipt[]}; - -export interface ReceiptContent { - [eventId: string]: { - [key in ReceiptType]: { - [userId: string]: Receipt; - }; - }; -} - const ReceiptPairRealIndex = 0; const ReceiptPairSyntheticIndex = 1; -// We will only hold a synthetic receipt if we do not have a real receipt or the synthetic is newer. -type Receipts = { - [receiptType: string]: { - [userId: string]: [WrappedReceipt | null, WrappedReceipt | null]; // Pair (both nullable) - }; -}; export abstract class ReadReceipt< Events extends string, diff --git a/src/models/room.ts b/src/models/room.ts index 51dd38913ef..21b06a6ced6 100644 --- a/src/models/room.ts +++ b/src/models/room.ts @@ -54,14 +54,11 @@ import { FILTER_RELATED_BY_SENDERS, ThreadFilterType, } from "./thread"; -import { ReceiptType } from "../@types/read_receipts"; +import { MAIN_ROOM_TIMELINE, Receipt, ReceiptContent, ReceiptType } from "../@types/read_receipts"; import { IStateEventWithRoomId } from "../@types/search"; import { RelationsContainer } from "./relations-container"; import { - MAIN_ROOM_TIMELINE, ReadReceipt, - Receipt, - ReceiptContent, synthesizeReceipt, } from "./read-receipt"; import { Feature, ServerSupport } from "../feature"; diff --git a/src/sync-accumulator.ts b/src/sync-accumulator.ts index e67ce9e6360..484e96e4f70 100644 --- a/src/sync-accumulator.ts +++ b/src/sync-accumulator.ts @@ -24,7 +24,7 @@ import { deepCopy, isSupportedReceiptType } from "./utils"; import { IContent, IUnsigned } from "./models/event"; import { IRoomSummary } from "./models/room-summary"; import { EventType } from "./@types/event"; -import { ReceiptType } from "./@types/read_receipts"; +import { MAIN_ROOM_TIMELINE, ReceiptContent, ReceiptType } from "./@types/read_receipts"; import { UNREAD_THREAD_NOTIFICATIONS } from './@types/sync'; interface IOpts { @@ -165,6 +165,15 @@ interface IRoom { eventId: string; }; }; + _threadReadReceipts: { + [threadId: string]: { + [userId: string]: { + data: IMinimalEvent; + type: ReceiptType; + eventId: string; + }; + }; + }; } export interface ISyncData { @@ -369,6 +378,7 @@ export class SyncAccumulator { _unreadThreadNotifications: {}, _summary: {}, _readReceipts: {}, + _threadReadReceipts: {}, }; } const currentData = this.joinRooms[roomId]; @@ -425,23 +435,30 @@ export class SyncAccumulator { // of a hassle to work with. We'll inflate this back out when // getJSON() is called. Object.keys(e.content).forEach((eventId) => { - Object.entries<{ - [eventId: string]: { - [receiptType: string]: { - [userId: string]: IMinimalEvent; - }; - }; - }>(e.content[eventId]).forEach(([key, value]) => { + Object.entries(e.content[eventId]).forEach(([key, value]) => { if (!isSupportedReceiptType(key)) return; - Object.keys(value).forEach((userId) => { - // clobber on user ID - currentData._readReceipts[userId] = { + for (const userId of Object.keys(value)) { + const data = e.content[eventId][key][userId]; + + const receipt = { data: e.content[eventId][key][userId], type: key as ReceiptType, eventId: eventId, }; - }); + + if (!data.thread_id || data.thread_id === MAIN_ROOM_TIMELINE) { + currentData._readReceipts[userId] = receipt; + } else { + currentData._threadReadReceipts = { + ...currentData._threadReadReceipts, + [data.thread_id]: { + ...(currentData._threadReadReceipts[data.thread_id] ?? {}), + [userId]: receipt, + }, + }; + } + } }); }); }); @@ -566,8 +583,8 @@ export class SyncAccumulator { // $event_id: { "m.read": { $user_id: $json } } }, }; - Object.keys(roomData._readReceipts).forEach((userId) => { - const receiptData = roomData._readReceipts[userId]; + + for (const [userId, receiptData] of Object.entries(roomData._readReceipts)) { if (!receiptEvent.content[receiptData.eventId]) { receiptEvent.content[receiptData.eventId] = {}; } @@ -577,7 +594,21 @@ export class SyncAccumulator { receiptEvent.content[receiptData.eventId][receiptData.type][userId] = ( receiptData.data ); - }); + } + + for (const threadReceipts of Object.values(roomData._threadReadReceipts)) { + for (const [userId, receiptData] of Object.entries(threadReceipts)) { + if (!receiptEvent.content[receiptData.eventId]) { + receiptEvent.content[receiptData.eventId] = {}; + } + if (!receiptEvent.content[receiptData.eventId][receiptData.type]) { + receiptEvent.content[receiptData.eventId][receiptData.type] = {}; + } + receiptEvent.content[receiptData.eventId][receiptData.type][userId] = ( + receiptData.data + ); + } + } // add only if we have some receipt data if (Object.keys(receiptEvent.content).length > 0) { roomJson.ephemeral.events.push(receiptEvent as IMinimalEvent);