diff --git a/spec/unit/matrix-client.spec.ts b/spec/unit/matrix-client.spec.ts index a23d14ba648..24f9d55e348 100644 --- a/spec/unit/matrix-client.spec.ts +++ b/spec/unit/matrix-client.spec.ts @@ -29,6 +29,7 @@ import { import { MEGOLM_ALGORITHM } from "../../src/crypto/olmlib"; import { EventStatus, MatrixEvent } from "../../src/models/event"; import { Preset } from "../../src/@types/partials"; +import { ReceiptType } from "../../src/@types/read_receipts"; import * as testUtils from "../test-utils/test-utils"; import { makeBeaconInfoContent } from "../../src/content-helpers"; import { M_BEACON_INFO } from "../../src/@types/beacon"; @@ -992,6 +993,46 @@ describe("MatrixClient", function() { }); }); + describe("read-markers and read-receipts", () => { + it("setRoomReadMarkers", () => { + client.setRoomReadMarkersHttpRequest = jest.fn(); + const room = { + hasPendingEvent: jest.fn().mockReturnValue(false), + addLocalEchoReceipt: jest.fn(), + }; + const rrEvent = new MatrixEvent({ event_id: "read_event_id" }); + const rpEvent = new MatrixEvent({ event_id: "read_private_event_id" }); + client.getRoom = () => room; + + client.setRoomReadMarkers( + "room_id", + "read_marker_event_id", + rrEvent, + rpEvent, + ); + + expect(client.setRoomReadMarkersHttpRequest).toHaveBeenCalledWith( + "room_id", + "read_marker_event_id", + "read_event_id", + "read_private_event_id", + ); + expect(room.addLocalEchoReceipt).toHaveBeenCalledTimes(2); + expect(room.addLocalEchoReceipt).toHaveBeenNthCalledWith( + 1, + client.credentials.userId, + rrEvent, + ReceiptType.Read, + ); + expect(room.addLocalEchoReceipt).toHaveBeenNthCalledWith( + 2, + client.credentials.userId, + rpEvent, + ReceiptType.ReadPrivate, + ); + }); + }); + describe("beacons", () => { const roomId = '!room:server.org'; const content = makeBeaconInfoContent(100, true); diff --git a/spec/unit/room.spec.ts b/spec/unit/room.spec.ts index abf839a15a0..a33fccfebe2 100644 --- a/spec/unit/room.spec.ts +++ b/spec/unit/room.spec.ts @@ -23,6 +23,7 @@ import * as utils from "../test-utils/test-utils"; import { DuplicateStrategy, EventStatus, + EventTimelineSet, EventType, JoinRule, MatrixEvent, @@ -31,11 +32,12 @@ import { RoomEvent, } from "../../src"; import { EventTimeline } from "../../src/models/event-timeline"; -import { Room } from "../../src/models/room"; +import { IWrappedReceipt, Room } from "../../src/models/room"; 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 { Thread, ThreadEvent } from "../../src/models/thread"; describe("Room", function() { @@ -2286,4 +2288,29 @@ describe("Room", function() { expect(responseRelations[0][1].has(threadReaction)).toBeTruthy(); }); }); + + describe("getEventReadUpTo()", () => { + const client = new TestClient(userA).client; + const room = new Room(roomId, client, userA); + + it("handles missing receipt type", () => { + room.getReadReceiptForUserId = (userId, ignore, receiptType) => { + return receiptType === ReceiptType.ReadPrivate ? { eventId: "eventId" } as IWrappedReceipt : null; + }; + + expect(room.getEventReadUpTo(userA)).toEqual("eventId"); + }); + + it("prefers older receipt", () => { + room.getReadReceiptForUserId = (userId, ignore, receiptType) => { + return (receiptType === ReceiptType.Read + ? { eventId: "eventId1" } + : { eventId: "eventId2" } + ) as IWrappedReceipt; + }; + room.getUnfilteredTimelineSet = () => ({ compareEventOrdering: (event1, event2) => 1 } as EventTimelineSet); + + expect(room.getEventReadUpTo(userA)).toEqual("eventId1"); + }); + }); }); diff --git a/spec/unit/sync-accumulator.spec.js b/spec/unit/sync-accumulator.spec.js index b089e0cebc1..5fe9a3611b0 100644 --- a/spec/unit/sync-accumulator.spec.js +++ b/spec/unit/sync-accumulator.spec.js @@ -15,6 +15,7 @@ See the License for the specific language governing permissions and limitations under the License. */ +import { ReceiptType } from "../../src/@types/read_receipts"; import { SyncAccumulator } from "../../src/sync-accumulator"; // The event body & unsigned object get frozen to assert that they don't get altered @@ -294,10 +295,13 @@ describe("SyncAccumulator", function() { room_id: "!foo:bar", content: { "$event1:localhost": { - "m.read": { + [ReceiptType.Read]: { "@alice:localhost": { ts: 1 }, "@bob:localhost": { ts: 2 }, }, + [ReceiptType.ReadPrivate]: { + "@dan:localhost": { ts: 4 }, + }, "some.other.receipt.type": { "@should_be_ignored:localhost": { key: "val" }, }, @@ -309,7 +313,7 @@ describe("SyncAccumulator", function() { room_id: "!foo:bar", content: { "$event2:localhost": { - "m.read": { + [ReceiptType.Read]: { "@bob:localhost": { ts: 2 }, // clobbers event1 receipt "@charlie:localhost": { ts: 3 }, }, @@ -337,12 +341,15 @@ describe("SyncAccumulator", function() { room_id: "!foo:bar", content: { "$event1:localhost": { - "m.read": { + [ReceiptType.Read]: { "@alice:localhost": { ts: 1 }, }, + [ReceiptType.ReadPrivate]: { + "@dan:localhost": { ts: 4 }, + }, }, "$event2:localhost": { - "m.read": { + [ReceiptType.Read]: { "@bob:localhost": { ts: 2 }, "@charlie:localhost": { ts: 3 }, }, diff --git a/src/@types/read_receipts.ts b/src/@types/read_receipts.ts new file mode 100644 index 00000000000..7a3ba268446 --- /dev/null +++ b/src/@types/read_receipts.ts @@ -0,0 +1,21 @@ +/* +Copyright 2022 Šimon Brandner + +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. +*/ + +export enum ReceiptType { + Read = "m.read", + FullyRead = "m.fully_read", + ReadPrivate = "org.matrix.msc2285.read.private" +} diff --git a/src/client.ts b/src/client.ts index 45ced3946f9..e772f7119d5 100644 --- a/src/client.ts +++ b/src/client.ts @@ -181,6 +181,7 @@ import { CryptoStore } from "./crypto/store/base"; import { MediaHandler } from "./webrtc/mediaHandler"; import { IRefreshTokenResponse } from "./@types/auth"; import { TypedEventEmitter } from "./models/typed-event-emitter"; +import { ReceiptType } from "./@types/read_receipts"; import { Thread, THREAD_RELATION_TYPE } from "./models/thread"; import { MBeaconInfoEventContent, M_BEACON_INFO } from "./@types/beacon"; @@ -1078,7 +1079,13 @@ export class MatrixClient extends TypedEventEmitter { - return Object.keys(content[eid]['m.read']).includes(this.getUserId()); + const read = content[eid][ReceiptType.Read]; + if (read && Object.keys(read).includes(this.getUserId())) return true; + + const readPrivate = content[eid][ReceiptType.ReadPrivate]; + if (readPrivate && Object.keys(readPrivate).includes(this.getUserId())) return true; + + return false; }).length > 0; if (!isSelf) return; @@ -4493,13 +4500,14 @@ export class MatrixClient extends TypedEventEmitter { + public sendReceipt(event: MatrixEvent, receiptType: ReceiptType, body: any, callback?: Callback): Promise<{}> { if (typeof (body) === 'function') { callback = body as any as Callback; // legacy body = {}; @@ -4526,32 +4534,19 @@ export class MatrixClient extends TypedEventEmitterThis - * property is unstable and may change in the future. + * @param {ReceiptType} receiptType other than ReceiptType.Read are experimental! Optional. * @param {module:client.callback} callback Optional. * @return {Promise} Resolves: to an empty object * @return {module:http-api.MatrixError} Rejects: with an error response. */ - public async sendReadReceipt(event: MatrixEvent, opts?: { hidden?: boolean }, callback?: Callback): Promise<{}> { - if (typeof (opts) === 'function') { - callback = opts as any as Callback; // legacy - opts = {}; - } - if (!opts) opts = {}; - + public async sendReadReceipt(event: MatrixEvent, receiptType = ReceiptType.Read, callback?: Callback): Promise<{}> { const eventId = event.getId(); const room = this.getRoom(event.getRoomId()); if (room && room.hasPendingEvent(eventId)) { throw new Error(`Cannot set read receipt to a pending event (${eventId})`); } - const addlContent = { - "org.matrix.msc2285.hidden": Boolean(opts.hidden), - }; - - return this.sendReceipt(event, "m.read", addlContent, callback); + return this.sendReceipt(event, receiptType, {}, callback); } /** @@ -4564,16 +4559,15 @@ export class MatrixClient extends TypedEventEmitterThis property is unstable and may change in the future. + * @param {MatrixEvent} rpEvent the m.read.private read receipt event for when we don't + * want other users to see the read receipts. This is experimental. Optional. * @return {Promise} Resolves: the empty object, {}. */ public async setRoomReadMarkers( roomId: string, rmEventId: string, - rrEvent: MatrixEvent, - opts: { hidden?: boolean }, + rrEvent?: MatrixEvent, + rpEvent?: MatrixEvent, ): Promise<{}> { const room = this.getRoom(roomId); if (room && room.hasPendingEvent(rmEventId)) { @@ -4581,18 +4575,26 @@ export class MatrixClient extends TypedEventEmitterThis - * property is currently unstable and may change in the future. + * @param {string} rpEventId rpEvent the m.read.private read receipt event for when we + * don't want other users to see the read receipts. This is experimental. Optional. * @return {Promise} Resolves: the empty object, {}. */ public setRoomReadMarkersHttpRequest( roomId: string, rmEventId: string, rrEventId: string, - opts: { hidden?: boolean }, + rpEventId: string, ): Promise<{}> { const path = utils.encodeUri("/rooms/$roomId/read_markers", { $roomId: roomId, }); const content = { - "m.fully_read": rmEventId, - "m.read": rrEventId, - "org.matrix.msc2285.hidden": Boolean(opts ? opts.hidden : false), + [ReceiptType.FullyRead]: rmEventId, + [ReceiptType.Read]: rrEventId, + [ReceiptType.ReadPrivate]: rpEventId, }; return this.http.authedRequest(undefined, Method.Post, path, undefined, content); diff --git a/src/models/room.ts b/src/models/room.ts index e0769097ecd..fc538a62aab 100644 --- a/src/models/room.ts +++ b/src/models/room.ts @@ -47,6 +47,7 @@ import { ThreadFilterType, } from "./thread"; import { TypedEventEmitter } from "./typed-event-emitter"; +import { ReceiptType } from "../@types/read_receipts"; import { IStateEventWithRoomId } from "../@types/search"; // These constants are used as sane defaults when the homeserver doesn't support @@ -58,7 +59,7 @@ import { IStateEventWithRoomId } from "../@types/search"; const KNOWN_SAFE_ROOM_VERSION = '9'; const SAFE_ROOM_VERSIONS = ['1', '2', '3', '4', '5', '6', '7', '8', '9']; -function synthesizeReceipt(userId: string, event: MatrixEvent, receiptType: string): MatrixEvent { +function synthesizeReceipt(userId: string, event: MatrixEvent, receiptType: ReceiptType): MatrixEvent { // console.log("synthesizing receipt for "+event.getId()); return new MatrixEvent({ content: { @@ -93,13 +94,13 @@ interface IReceipt { ts: number; } -interface IWrappedReceipt { +export interface IWrappedReceipt { eventId: string; data: IReceipt; } interface ICachedReceipt { - type: string; + type: ReceiptType; userId: string; data: IReceipt; } @@ -108,7 +109,7 @@ type ReceiptCache = {[eventId: string]: ICachedReceipt[]}; interface IReceiptContent { [eventId: string]: { - [type: string]: { + [key in ReceiptType]: { [userId: string]: IReceipt; }; }; @@ -1792,7 +1793,7 @@ export class Room extends TypedEventEmitter // Don't synthesize RR for m.room.redaction as this causes the RR to go missing. if (event.sender && event.getType() !== EventType.RoomRedaction) { this.addReceipt(synthesizeReceipt( - event.sender.userId, event, "m.read", + event.sender.userId, event, ReceiptType.Read, ), true); // Any live events from a user could be taken as implicit @@ -2314,14 +2315,23 @@ export class Room extends TypedEventEmitter */ public getUsersReadUpTo(event: MatrixEvent): string[] { return this.getReceiptsForEvent(event).filter(function(receipt) { - return receipt.type === "m.read"; + return [ReceiptType.Read, ReceiptType.ReadPrivate].includes(receipt.type); }).map(function(receipt) { return receipt.userId; }); } - public getReadReceiptForUserId(userId: string, ignoreSynthesized = false): IWrappedReceipt | null { - const [realReceipt, syntheticReceipt] = this.receipts["m.read"]?.[userId] ?? []; + /** + * Gets the latest receipt for a given user in the room + * @param userId The id of the user for which we want the receipt + * @param ignoreSynthesized Whether to ignore synthesized receipts or not + * @param receiptType Optional. The type of the receipt we want to get + * @returns the latest receipts of the chosen type for the chosen user + */ + public getReadReceiptForUserId( + userId: string, ignoreSynthesized = false, receiptType = ReceiptType.Read, + ): IWrappedReceipt | null { + const [realReceipt, syntheticReceipt] = this.receipts[receiptType]?.[userId] ?? []; if (ignoreSynthesized) { return realReceipt; } @@ -2339,8 +2349,25 @@ export class Room extends TypedEventEmitter * @return {String} ID of the latest event that the given user has read, or null. */ public getEventReadUpTo(userId: string, ignoreSynthesized = false): string | null { - const readReceipt = this.getReadReceiptForUserId(userId, ignoreSynthesized); - return readReceipt?.eventId ?? null; + const timelineSet = this.getUnfilteredTimelineSet(); + const publicReadReceipt = this.getReadReceiptForUserId(userId, ignoreSynthesized, ReceiptType.Read); + const privateReadReceipt = this.getReadReceiptForUserId(userId, ignoreSynthesized, ReceiptType.ReadPrivate); + + // If we have both, compare them + let comparison: number | undefined; + if (publicReadReceipt?.eventId && privateReadReceipt?.eventId) { + comparison = timelineSet.compareEventOrdering(publicReadReceipt?.eventId, privateReadReceipt?.eventId); + } + + // If we didn't get a comparison try to compare the ts of the receipts + if (!comparison) comparison = publicReadReceipt?.data?.ts - privateReadReceipt?.data?.ts; + + // The public receipt is more likely to drift out of date so the private + // one has precedence + if (!comparison) return privateReadReceipt?.eventId ?? publicReadReceipt?.eventId ?? null; + + // If public read receipt is older, return the private one + return (comparison < 0) ? privateReadReceipt?.eventId : publicReadReceipt?.eventId; } /** @@ -2493,7 +2520,7 @@ export class Room extends TypedEventEmitter } this.receiptCacheByEventId[eventId].push({ userId: userId, - type: receiptType, + type: receiptType as ReceiptType, data: receipt, }); }); @@ -2506,9 +2533,9 @@ export class Room extends TypedEventEmitter * client the fact that we've sent one. * @param {string} userId The user ID if the receipt sender * @param {MatrixEvent} e The event that is to be acknowledged - * @param {string} receiptType The type of receipt + * @param {ReceiptType} receiptType The type of receipt */ - public addLocalEchoReceipt(userId: string, e: MatrixEvent, receiptType: string): void { + public addLocalEchoReceipt(userId: string, e: MatrixEvent, receiptType: ReceiptType): void { this.addReceipt(synthesizeReceipt(userId, e, receiptType), true); } diff --git a/src/sync-accumulator.ts b/src/sync-accumulator.ts index f50f80b566a..3f307a23355 100644 --- a/src/sync-accumulator.ts +++ b/src/sync-accumulator.ts @@ -24,6 +24,7 @@ import { deepCopy } 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"; interface IOpts { maxTimelineEntries?: number; @@ -157,6 +158,7 @@ interface IRoom { _readReceipts: { [userId: string]: { data: IMinimalEvent; + type: ReceiptType; eventId: string; }; }; @@ -416,16 +418,31 @@ 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) => { - if (!e.content[eventId]["m.read"]) { + if (!e.content[eventId][ReceiptType.Read] && !e.content[eventId][ReceiptType.ReadPrivate]) { return; } - Object.keys(e.content[eventId]["m.read"]).forEach((userId) => { - // clobber on user ID - currentData._readReceipts[userId] = { - data: e.content[eventId]["m.read"][userId], - eventId: eventId, - }; - }); + const read = e.content[eventId][ReceiptType.Read]; + if (read) { + Object.keys(read).forEach((userId) => { + // clobber on user ID + currentData._readReceipts[userId] = { + data: e.content[eventId][ReceiptType.Read][userId], + type: ReceiptType.Read, + eventId: eventId, + }; + }); + } + const readPrivate = e.content[eventId][ReceiptType.ReadPrivate]; + if (readPrivate) { + Object.keys(readPrivate).forEach((userId) => { + // clobber on user ID + currentData._readReceipts[userId] = { + data: e.content[eventId][ReceiptType.ReadPrivate][userId], + type: ReceiptType.ReadPrivate, + eventId: eventId, + }; + }); + } }); }); } @@ -552,11 +569,12 @@ export class SyncAccumulator { Object.keys(roomData._readReceipts).forEach((userId) => { const receiptData = roomData._readReceipts[userId]; if (!receiptEvent.content[receiptData.eventId]) { - receiptEvent.content[receiptData.eventId] = { - "m.read": {}, - }; + receiptEvent.content[receiptData.eventId] = {}; + } + if (!receiptEvent.content[receiptData.eventId][receiptData.type]) { + receiptEvent.content[receiptData.eventId][receiptData.type] = {}; } - receiptEvent.content[receiptData.eventId]["m.read"][userId] = ( + receiptEvent.content[receiptData.eventId][receiptData.type][userId] = ( receiptData.data ); });