From 815370c5f95a922dc29e04dddbc7abe274399ca3 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Fri, 25 Nov 2022 13:21:16 +0000 Subject: [PATCH 1/6] sliding sync: add receipts extension --- spec/integ/sliding-sync-sdk.spec.ts | 91 +++++++++++++++++++++++++++++ src/sliding-sync-sdk.ts | 41 +++++++++++++ 2 files changed, 132 insertions(+) diff --git a/spec/integ/sliding-sync-sdk.spec.ts b/spec/integ/sliding-sync-sdk.spec.ts index 3a43f9e12..5246e12aa 100644 --- a/spec/integ/sliding-sync-sdk.spec.ts +++ b/spec/integ/sliding-sync-sdk.spec.ts @@ -928,4 +928,95 @@ describe("SlidingSyncSdk", () => { expect(room.getMember(selfUserId)?.typing).toEqual(false); }); }); + + describe("ExtensionReceipts", () => { + let ext: Extension; + + beforeAll(async () => { + await setupClient(); + const hasSynced = sdk!.sync(); + await httpBackend!.flushAllExpected(); + await hasSynced; + ext = findExtension("receipts"); + }); + + it("gets enabled on the initial request only", () => { + expect(ext.onRequest(true)).toEqual({ + enabled: true, + }); + expect(ext.onRequest(false)).toEqual(undefined); + }); + + it("processes receipts", async () => { + const roomId = "!room:id"; + const alice = "@alice:alice"; + const lastEvent = mkOwnEvent(EventType.RoomMessage, { body: "hello" }); + mockSlidingSync!.emit(SlidingSyncEvent.RoomData, roomId, { + name: "Room with receipts", + required_state: [], + timeline: [ + mkOwnStateEvent(EventType.RoomCreate, { creator: selfUserId }, ""), + mkOwnStateEvent(EventType.RoomMember, { membership: "join" }, selfUserId), + mkOwnStateEvent(EventType.RoomPowerLevels, { users: { [selfUserId]: 100 } }, ""), + { + type: EventType.RoomMember, + state_key: alice, + content: {membership: "join"}, + sender: alice, + origin_server_ts: Date.now(), + event_id: "$alice", + }, + lastEvent, + ], + initial: true, + }); + const room = client!.getRoom(roomId)!; + expect(room).toBeDefined(); + expect(room.getReadReceiptForUserId(alice, true)).toBeNull(); + ext.onResponse({ + rooms: { + [roomId]: { + type: EventType.Receipt, + content: { + [lastEvent.event_id]: { + "m.read": { + [alice]: { + ts: 1234567, + } + }, + }, + }, + }, + }, + }); + const receipt = room.getReadReceiptForUserId(alice); + expect(receipt).toBeDefined(); + expect(receipt?.eventId).toEqual(lastEvent.event_id); + expect(receipt?.data.ts).toEqual(1234567); + expect(receipt?.data.thread_id).toBeFalsy(); + }); + + it("gracefully handles missing rooms when receiving receipts", async () => { + const roomId = "!room:id"; + const alice = "@alice:alice"; + const eventId = "$something"; + ext.onResponse({ + rooms: { + [roomId]: { + type: EventType.Receipt, + content: { + [eventId]: { + "m.read": { + [alice]: { + ts: 1234567, + } + }, + }, + }, + }, + }, + }); + // we expect it not to crash + }); + }); }); diff --git a/src/sliding-sync-sdk.ts b/src/sliding-sync-sdk.ts index f12b51c65..5dc10bb6f 100644 --- a/src/sliding-sync-sdk.ts +++ b/src/sliding-sync-sdk.ts @@ -273,6 +273,46 @@ class ExtensionTyping implements Extension { } } +class ExtensionReceipts implements Extension { + public constructor(private readonly client: MatrixClient) {} + + public name(): string { + return "receipts"; + } + + public when(): ExtensionState { + return ExtensionState.PostProcess; + } + + public onRequest(isInitial: boolean): object | undefined { + if (!isInitial) { + return undefined; + } + return { + enabled: true, + }; + } + + public onResponse(data: {rooms: Record}): void { + if (!data || !data.rooms) { + return; + } + + for (const roomId in data.rooms) { + const ephemeralEvents = mapEvents(this.client, roomId, [data.rooms[roomId]]); + const room = this.client.getRoom(roomId); + if (!room) { + logger.warn("got receipts for room but room doesn't exist on client:", roomId); + continue; + } + room.addEphemeralEvents(ephemeralEvents); + ephemeralEvents.forEach((e) => { + this.client.emit(ClientEvent.Event, e); + }); + } + } +} + /** * A copy of SyncApi such that it can be used as a drop-in replacement for sync v2. For the actual * sliding sync API, see sliding-sync.ts or the class SlidingSync. @@ -314,6 +354,7 @@ export class SlidingSyncSdk { new ExtensionToDevice(this.client), new ExtensionAccountData(this.client), new ExtensionTyping(this.client), + new ExtensionReceipts(this.client), ]; if (this.opts.crypto) { extensions.push( From 74147b9943f9371da87e5df9d5db52bd9f067249 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Fri, 25 Nov 2022 13:24:12 +0000 Subject: [PATCH 2/6] Linting --- spec/integ/sliding-sync-sdk.spec.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/integ/sliding-sync-sdk.spec.ts b/spec/integ/sliding-sync-sdk.spec.ts index 5246e12aa..6945404dd 100644 --- a/spec/integ/sliding-sync-sdk.spec.ts +++ b/spec/integ/sliding-sync-sdk.spec.ts @@ -961,7 +961,7 @@ describe("SlidingSyncSdk", () => { { type: EventType.RoomMember, state_key: alice, - content: {membership: "join"}, + content: { membership: "join" }, sender: alice, origin_server_ts: Date.now(), event_id: "$alice", @@ -982,7 +982,7 @@ describe("SlidingSyncSdk", () => { "m.read": { [alice]: { ts: 1234567, - } + }, }, }, }, @@ -1009,7 +1009,7 @@ describe("SlidingSyncSdk", () => { "m.read": { [alice]: { ts: 1234567, - } + }, }, }, }, From fc91153be42f97c8c4b302e5b3a9f5cc750ac9ad Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 28 Nov 2022 10:23:23 +0000 Subject: [PATCH 3/6] Revert "Process `m.room.encryption` events before emitting `RoomMember` events" (#2913) This reverts commit aaf3702c66011a4352332d1ad4dfb15f8b10ce49. --- spec/integ/megolm-integ.spec.ts | 107 ++------------------------------ spec/test-utils/test-utils.ts | 24 +++---- src/crypto/index.ts | 2 +- src/sync.ts | 37 +++++------ 4 files changed, 35 insertions(+), 135 deletions(-) diff --git a/spec/integ/megolm-integ.spec.ts b/spec/integ/megolm-integ.spec.ts index 16ecf40bb..a4891b702 100644 --- a/spec/integ/megolm-integ.spec.ts +++ b/spec/integ/megolm-integ.spec.ts @@ -21,19 +21,16 @@ import * as testUtils from "../test-utils/test-utils"; import { TestClient } from "../TestClient"; import { logger } from "../../src/logger"; import { - IClaimOTKsResult, IContent, - IDownloadKeyResult, IEvent, + IClaimOTKsResult, IJoinedRoom, - IndexedDBCryptoStore, ISyncResponse, - IUploadKeysRequest, + IDownloadKeyResult, MatrixEvent, MatrixEventEvent, + IndexedDBCryptoStore, Room, - RoomMember, - RoomStateEvent, } from "../../src/matrix"; import { IDeviceKeys } from "../../src/crypto/dehydration"; import { DeviceInfo } from "../../src/crypto/deviceinfo"; @@ -330,9 +327,7 @@ describe("megolm", () => { const room = aliceTestClient.client.getRoom(ROOM_ID)!; const event = room.getLiveTimeline().getEvents()[0]; expect(event.isEncrypted()).toBe(true); - - // it probably won't be decrypted yet, because it takes a while to process the olm keys - const decryptedEvent = await testUtils.awaitDecryption(event, { waitOnDecryptionFailure: true }); + const decryptedEvent = await testUtils.awaitDecryption(event); expect(decryptedEvent.getContent().body).toEqual('42'); }); @@ -878,12 +873,7 @@ describe("megolm", () => { const room = aliceTestClient.client.getRoom(ROOM_ID)!; await room.decryptCriticalEvents(); - - // it probably won't be decrypted yet, because it takes a while to process the olm keys - const decryptedEvent = await testUtils.awaitDecryption( - room.getLiveTimeline().getEvents()[0], { waitOnDecryptionFailure: true }, - ); - expect(decryptedEvent.getContent().body).toEqual('42'); + expect(room.getLiveTimeline().getEvents()[0].getContent().body).toEqual('42'); const exported = await aliceTestClient.client.exportRoomKeys(); @@ -1022,9 +1012,7 @@ describe("megolm", () => { const room = aliceTestClient.client.getRoom(ROOM_ID)!; const event = room.getLiveTimeline().getEvents()[0]; expect(event.isEncrypted()).toBe(true); - - // it probably won't be decrypted yet, because it takes a while to process the olm keys - const decryptedEvent = await testUtils.awaitDecryption(event, { waitOnDecryptionFailure: true }); + const decryptedEvent = await testUtils.awaitDecryption(event); expect(decryptedEvent.getRoomId()).toEqual(ROOM_ID); expect(decryptedEvent.getContent()).toEqual({}); expect(decryptedEvent.getClearContent()).toBeUndefined(); @@ -1376,87 +1364,4 @@ describe("megolm", () => { await beccaTestClient.stop(); }); - - it("allows enabling encryption in the createRoom call", async () => { - const testRoomId = "!testRoom:id"; - await aliceTestClient.start(); - - aliceTestClient.httpBackend.when("POST", "/keys/query") - .respond(200, function(_path, content: IUploadKeysRequest) { - return { device_keys: {} }; - }); - - /* Alice makes the /createRoom call */ - aliceTestClient.httpBackend.when("POST", "/createRoom") - .respond(200, { room_id: testRoomId }); - await Promise.all([ - aliceTestClient.client.createRoom({ - initial_state: [{ - type: 'm.room.encryption', - state_key: '', - content: { algorithm: 'm.megolm.v1.aes-sha2' }, - }], - }), - aliceTestClient.httpBackend.flushAllExpected(), - ]); - - /* The sync arrives in two parts; first the m.room.create... */ - aliceTestClient.httpBackend.when("GET", "/sync").respond(200, { - rooms: { join: { - [testRoomId]: { - timeline: { events: [ - { - type: 'm.room.create', - state_key: '', - event_id: "$create", - }, - { - type: 'm.room.member', - state_key: aliceTestClient.getUserId(), - content: { membership: "join" }, - event_id: "$alijoin", - }, - ] }, - }, - } }, - }); - await aliceTestClient.flushSync(); - - // ... and then the e2e event and an invite ... - aliceTestClient.httpBackend.when("GET", "/sync").respond(200, { - rooms: { join: { - [testRoomId]: { - timeline: { events: [ - { - type: 'm.room.encryption', - state_key: '', - content: { algorithm: 'm.megolm.v1.aes-sha2' }, - event_id: "$e2e", - }, - { - type: 'm.room.member', - state_key: "@other:user", - content: { membership: "invite" }, - event_id: "$otherinvite", - }, - ] }, - }, - } }, - }); - - // as soon as the roomMember arrives, try to send a message - aliceTestClient.client.on(RoomStateEvent.NewMember, (_e, _s, member: RoomMember) => { - if (member.userId == "@other:user") { - aliceTestClient.client.sendMessage(testRoomId, { msgtype: "m.text", body: "Hello, World" }); - } - }); - - // flush the sync and wait for the /send/ request. - aliceTestClient.httpBackend.when("PUT", "/send/m.room.encrypted/") - .respond(200, (_path, _content) => ({ event_id: "asdfgh" })); - await Promise.all([ - aliceTestClient.flushSync(), - aliceTestClient.httpBackend.flush("/send/m.room.encrypted/", 1), - ]); - }); }); diff --git a/spec/test-utils/test-utils.ts b/spec/test-utils/test-utils.ts index 21ae4d18b..af87ebbe6 100644 --- a/spec/test-utils/test-utils.ts +++ b/spec/test-utils/test-utils.ts @@ -362,28 +362,22 @@ export class MockStorageApi { * @param {MatrixEvent} event * @returns {Promise} promise which resolves (to `event`) when the event has been decrypted */ -export async function awaitDecryption( - event: MatrixEvent, { waitOnDecryptionFailure = false } = {}, -): Promise { +export async function awaitDecryption(event: MatrixEvent): Promise { // An event is not always decrypted ahead of time // getClearContent is a good signal to know whether an event has been decrypted // already if (event.getClearContent() !== null) { - if (waitOnDecryptionFailure && event.isDecryptionFailure()) { - logger.log(`${Date.now()} event ${event.getId()} got decryption error; waiting`); - } else { - return event; - } + return event; } else { - logger.log(`${Date.now()} event ${event.getId()} is not yet decrypted; waiting`); - } + logger.log(`${Date.now()} event ${event.getId()} is being decrypted; waiting`); - return new Promise((resolve) => { - event.once(MatrixEventEvent.Decrypted, (ev) => { - logger.log(`${Date.now()} event ${event.getId()} now decrypted`); - resolve(ev); + return new Promise((resolve) => { + event.once(MatrixEventEvent.Decrypted, (ev) => { + logger.log(`${Date.now()} event ${event.getId()} now decrypted`); + resolve(ev); + }); }); - }); + } } export const emitPromise = (e: EventEmitter, k: string): Promise => new Promise(r => e.once(k, r)); diff --git a/src/crypto/index.ts b/src/crypto/index.ts index 3e7954f68..4a7f73e9b 100644 --- a/src/crypto/index.ts +++ b/src/crypto/index.ts @@ -2815,7 +2815,7 @@ export class Crypto extends TypedEventEmitterreject->invite cycles @@ -1361,18 +1362,6 @@ export class SyncApi { } } - // process any crypto events *before* emitting the RoomStateEvent events. This - // avoids a race condition if the application tries to send a message after the - // state event is processed, but before crypto is enabled, which then causes the - // crypto layer to complain. - if (this.opts.crypto) { - for (const e of stateEvents.concat(events)) { - if (e.isState() && e.getType() === EventType.RoomEncryption && e.getStateKey() === "") { - await this.opts.crypto.onCryptoEvent(e); - } - } - } - try { await this.injectRoomEvents(room, stateEvents, events, syncEventData.fromCache); } catch (e) { @@ -1394,16 +1383,27 @@ export class SyncApi { room.recalculate(); if (joinObj.isBrandNewRoom) { + client.store.storeRoom(room); client.emit(ClientEvent.Room, room); } this.processEventsForNotifs(room, events); - const emitEvent = (e: MatrixEvent): boolean => client.emit(ClientEvent.Event, e); - stateEvents.forEach(emitEvent); - events.forEach(emitEvent); - ephemeralEvents.forEach(emitEvent); - accountDataEvents.forEach(emitEvent); + const processRoomEvent = async (e): Promise => { + client.emit(ClientEvent.Event, e); + if (e.isState() && e.getType() == "m.room.encryption" && this.opts.crypto) { + await this.opts.crypto.onCryptoEvent(e); + } + }; + + await utils.promiseMapSeries(stateEvents, processRoomEvent); + await utils.promiseMapSeries(events, processRoomEvent); + ephemeralEvents.forEach(function(e) { + client.emit(ClientEvent.Event, e); + }); + accountDataEvents.forEach(function(e) { + client.emit(ClientEvent.Event, e); + }); // Decrypt only the last message in all rooms to make sure we can generate a preview // And decrypt all events after the recorded read receipt to ensure an accurate @@ -1423,6 +1423,7 @@ export class SyncApi { room.recalculate(); if (leaveObj.isBrandNewRoom) { + client.store.storeRoom(room); client.emit(ClientEvent.Room, room); } From 6592b2c2051bd6413e8ffcf1882f626e65b2628d Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Mon, 28 Nov 2022 10:58:38 +0000 Subject: [PATCH 4/6] sonarcloud --- spec/integ/sliding-sync-sdk.spec.ts | 59 +++++++++++++---------------- src/sliding-sync-sdk.ts | 53 +++++++++++++------------- 2 files changed, 53 insertions(+), 59 deletions(-) diff --git a/spec/integ/sliding-sync-sdk.spec.ts b/spec/integ/sliding-sync-sdk.spec.ts index 6945404dd..72a7eeaa2 100644 --- a/spec/integ/sliding-sync-sdk.spec.ts +++ b/spec/integ/sliding-sync-sdk.spec.ts @@ -932,6 +932,27 @@ describe("SlidingSyncSdk", () => { describe("ExtensionReceipts", () => { let ext: Extension; + const generateReceiptResponse = ( + userId: string, roomId: string, eventId: string, recType: string, ts: number, + ) => { + return { + rooms: { + [roomId]: { + type: EventType.Receipt, + content: { + [eventId]: { + [recType]: { + [userId]: { + ts: ts, + }, + }, + }, + }, + }, + }, + }; + }; + beforeAll(async () => { await setupClient(); const hasSynced = sdk!.sync(); @@ -973,22 +994,9 @@ describe("SlidingSyncSdk", () => { const room = client!.getRoom(roomId)!; expect(room).toBeDefined(); expect(room.getReadReceiptForUserId(alice, true)).toBeNull(); - ext.onResponse({ - rooms: { - [roomId]: { - type: EventType.Receipt, - content: { - [lastEvent.event_id]: { - "m.read": { - [alice]: { - ts: 1234567, - }, - }, - }, - }, - }, - }, - }); + ext.onResponse( + generateReceiptResponse(alice, roomId, lastEvent.event_id, "m.read", 1234567), + ); const receipt = room.getReadReceiptForUserId(alice); expect(receipt).toBeDefined(); expect(receipt?.eventId).toEqual(lastEvent.event_id); @@ -1000,22 +1008,9 @@ describe("SlidingSyncSdk", () => { const roomId = "!room:id"; const alice = "@alice:alice"; const eventId = "$something"; - ext.onResponse({ - rooms: { - [roomId]: { - type: EventType.Receipt, - content: { - [eventId]: { - "m.read": { - [alice]: { - ts: 1234567, - }, - }, - }, - }, - }, - }, - }); + ext.onResponse( + generateReceiptResponse(alice, roomId, eventId, "m.read", 1234567), + ); // we expect it not to crash }); }); diff --git a/src/sliding-sync-sdk.ts b/src/sliding-sync-sdk.ts index 5dc10bb6f..ae6000667 100644 --- a/src/sliding-sync-sdk.ts +++ b/src/sliding-sync-sdk.ts @@ -253,22 +253,15 @@ class ExtensionTyping implements Extension { }; } - public onResponse(data: {rooms: Record}): void { + public onResponse(data: {rooms: Record}): void { if (!data || !data.rooms) { return; } for (const roomId in data.rooms) { - const ephemeralEvents = mapEvents(this.client, roomId, [data.rooms[roomId]]); - const room = this.client.getRoom(roomId); - if (!room) { - logger.warn("got typing events for room but room doesn't exist on client:", roomId); - continue; - } - room.addEphemeralEvents(ephemeralEvents); - ephemeralEvents.forEach((e) => { - this.client.emit(ClientEvent.Event, e); - }); + processEphemeralEvents( + this.client, roomId, [data.rooms[roomId]], + ); } } } @@ -285,30 +278,23 @@ class ExtensionReceipts implements Extension { } public onRequest(isInitial: boolean): object | undefined { - if (!isInitial) { - return undefined; + if (isInitial) { + return { + enabled: true, + }; } - return { - enabled: true, - }; + return undefined; } - public onResponse(data: {rooms: Record}): void { + public onResponse(data: {rooms: Record}): void { if (!data || !data.rooms) { return; } for (const roomId in data.rooms) { - const ephemeralEvents = mapEvents(this.client, roomId, [data.rooms[roomId]]); - const room = this.client.getRoom(roomId); - if (!room) { - logger.warn("got receipts for room but room doesn't exist on client:", roomId); - continue; - } - room.addEphemeralEvents(ephemeralEvents); - ephemeralEvents.forEach((e) => { - this.client.emit(ClientEvent.Event, e); - }); + processEphemeralEvents( + this.client, roomId, [data.rooms[roomId]], + ); } } } @@ -970,3 +956,16 @@ function mapEvents(client: MatrixClient, roomId: string | undefined, events: obj return mapper(e); }); } + +function processEphemeralEvents(client: MatrixClient, roomId: string, ephEvents: IMinimalEvent[]) { + const ephemeralEvents = mapEvents(client, roomId, ephEvents); + const room = client.getRoom(roomId); + if (!room) { + logger.warn("got ephemeral events for room but room doesn't exist on client:", roomId); + return; + } + room.addEphemeralEvents(ephemeralEvents); + ephemeralEvents.forEach((e) => { + client.emit(ClientEvent.Event, e); + }); +} From c8c39052a7d4bc1b82b06dac9f2dc79696e4118d Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Mon, 28 Nov 2022 11:09:03 +0000 Subject: [PATCH 5/6] Linting --- src/sliding-sync-sdk.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sliding-sync-sdk.ts b/src/sliding-sync-sdk.ts index ae6000667..0c5524cbd 100644 --- a/src/sliding-sync-sdk.ts +++ b/src/sliding-sync-sdk.ts @@ -957,7 +957,7 @@ function mapEvents(client: MatrixClient, roomId: string | undefined, events: obj }); } -function processEphemeralEvents(client: MatrixClient, roomId: string, ephEvents: IMinimalEvent[]) { +function processEphemeralEvents(client: MatrixClient, roomId: string, ephEvents: IMinimalEvent[]): void { const ephemeralEvents = mapEvents(client, roomId, ephEvents); const room = client.getRoom(roomId); if (!room) { From 847766c114ab2e407e729338ff70d5f2a2c969bb Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Mon, 28 Nov 2022 18:13:17 +0000 Subject: [PATCH 6/6] Review comments --- src/sliding-sync-sdk.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sliding-sync-sdk.ts b/src/sliding-sync-sdk.ts index 0c5524cbd..a787b4e34 100644 --- a/src/sliding-sync-sdk.ts +++ b/src/sliding-sync-sdk.ts @@ -246,7 +246,7 @@ class ExtensionTyping implements Extension { public onRequest(isInitial: boolean): object | undefined { if (!isInitial) { - return undefined; + return undefined; // don't send a JSON object for subsequent requests, we don't need to. } return { enabled: true, @@ -283,7 +283,7 @@ class ExtensionReceipts implements Extension { enabled: true, }; } - return undefined; + return undefined; // don't send a JSON object for subsequent requests, we don't need to. } public onResponse(data: {rooms: Record}): void {