Merge branch 'develop' into kegan/sync-v3

kegsay · kegsay · commit a3248c0aa17b · 2022-08-08T12:51:03.000+01:00
diff --git a/spec/unit/queueToDevice.spec.ts b/spec/unit/queueToDevice.spec.ts
@@ -244,7 +244,8 @@ describe.each([
 
         client.retryImmediately();
 
-        expect(await httpBackend.flush(null, 1, 20)).toEqual(1);
+        // longer timeout here to try & avoid flakiness
+        expect(await httpBackend.flush(null, 1, 3000)).toEqual(1);
     });
 
     it("retries on when client is started", async function() {
diff --git a/spec/unit/room.spec.ts b/spec/unit/room.spec.ts
@@ -2435,16 +2435,96 @@ describe("Room", function() {
             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);
+        describe("prefers newer receipt", () => {
+            it("should compare correctly using timelines", () => {
+                room.getReadReceiptForUserId = (userId, ignore, receiptType) => {
+                    if (receiptType === ReceiptType.ReadPrivate) {
+                        return { eventId: "eventId1" } as IWrappedReceipt;
+                    }
+                    if (receiptType === ReceiptType.UnstableReadPrivate) {
+                        return { eventId: "eventId2" } as IWrappedReceipt;
+                    }
+                    if (receiptType === ReceiptType.Read) {
+                        return { eventId: "eventId3" } as IWrappedReceipt;
+                    }
+                };
+
+                for (let i = 1; i <= 3; i++) {
+                    room.getUnfilteredTimelineSet = () => ({ compareEventOrdering: (event1, event2) => {
+                        return (event1 === `eventId${i}`) ? 1 : -1;
+                    } } as EventTimelineSet);
+
+                    expect(room.getEventReadUpTo(userA)).toEqual(`eventId${i}`);
+                }
+            });
+
+            it("should compare correctly by timestamp", () => {
+                for (let i = 1; i <= 3; i++) {
+                    room.getUnfilteredTimelineSet = () => ({
+                        compareEventOrdering: (_1, _2) => null,
+                    } as EventTimelineSet);
+                    room.getReadReceiptForUserId = (userId, ignore, receiptType) => {
+                        if (receiptType === ReceiptType.ReadPrivate) {
+                            return { eventId: "eventId1", data: { ts: i === 1 ? 1 : 0 } } as IWrappedReceipt;
+                        }
+                        if (receiptType === ReceiptType.UnstableReadPrivate) {
+                            return { eventId: "eventId2", data: { ts: i === 2 ? 1 : 0 } } as IWrappedReceipt;
+                        }
+                        if (receiptType === ReceiptType.Read) {
+                            return { eventId: "eventId3", data: { ts: i === 3 ? 1 : 0 } } as IWrappedReceipt;
+                        }
+                    };
+
+                    expect(room.getEventReadUpTo(userA)).toEqual(`eventId${i}`);
+                }
+            });
 
-            expect(room.getEventReadUpTo(userA)).toEqual("eventId1");
+            describe("fallback precedence", () => {
+                beforeAll(() => {
+                    room.getUnfilteredTimelineSet = () => ({
+                        compareEventOrdering: (_1, _2) => null,
+                    } as EventTimelineSet);
+                });
+
+                it("should give precedence to m.read.private", () => {
+                    room.getReadReceiptForUserId = (userId, ignore, receiptType) => {
+                        if (receiptType === ReceiptType.ReadPrivate) {
+                            return { eventId: "eventId1" } as IWrappedReceipt;
+                        }
+                        if (receiptType === ReceiptType.UnstableReadPrivate) {
+                            return { eventId: "eventId2" } as IWrappedReceipt;
+                        }
+                        if (receiptType === ReceiptType.Read) {
+                            return { eventId: "eventId3" } as IWrappedReceipt;
+                        }
+                    };
+
+                    expect(room.getEventReadUpTo(userA)).toEqual(`eventId1`);
+                });
+
+                it("should give precedence to org.matrix.msc2285.read.private", () => {
+                    room.getReadReceiptForUserId = (userId, ignore, receiptType) => {
+                        if (receiptType === ReceiptType.UnstableReadPrivate) {
+                            return { eventId: "eventId2" } as IWrappedReceipt;
+                        }
+                        if (receiptType === ReceiptType.Read) {
+                            return { eventId: "eventId2" } as IWrappedReceipt;
+                        }
+                    };
+
+                    expect(room.getEventReadUpTo(userA)).toEqual(`eventId2`);
+                });
+
+                it("should give precedence to m.read", () => {
+                    room.getReadReceiptForUserId = (userId, ignore, receiptType) => {
+                        if (receiptType === ReceiptType.Read) {
+                            return { eventId: "eventId3" } as IWrappedReceipt;
+                        }
+                    };
+
+                    expect(room.getEventReadUpTo(userA)).toEqual(`eventId3`);
+                });
+            });
         });
     });
 });
diff --git a/spec/unit/sync-accumulator.spec.ts b/spec/unit/sync-accumulator.spec.ts
@@ -302,6 +302,9 @@ describe("SyncAccumulator", function() {
                     [ReceiptType.ReadPrivate]: {
                         "@dan:localhost": { ts: 4 },
                     },
+                    [ReceiptType.UnstableReadPrivate]: {
+                        "@matthew:localhost": { ts: 5 },
+                    },
                     "some.other.receipt.type": {
                         "@should_be_ignored:localhost": { key: "val" },
                     },
@@ -347,6 +350,9 @@ describe("SyncAccumulator", function() {
                     [ReceiptType.ReadPrivate]: {
                         "@dan:localhost": { ts: 4 },
                     },
+                    [ReceiptType.UnstableReadPrivate]: {
+                        "@matthew:localhost": { ts: 5 },
+                    },
                 },
                 "$event2:localhost": {
                     [ReceiptType.Read]: {
diff --git a/spec/unit/utils.spec.ts b/spec/unit/utils.spec.ts
@@ -15,6 +15,7 @@ import {
 import { logger } from "../../src/logger";
 import { mkMessage } from "../test-utils/test-utils";
 import { makeBeaconEvent } from "../test-utils/beacon";
+import { ReceiptType } from "../../src/@types/read_receipts";
 
 // TODO: Fix types throughout
 
@@ -523,4 +524,54 @@ describe("utils", function() {
             ).toEqual([beaconEvent2, beaconEvent1, beaconEvent3]);
         });
     });
+
+    describe('getPrivateReadReceiptField', () => {
+        it('should return m.read.private if server supports stable', async () => {
+            expect(await utils.getPrivateReadReceiptField({
+                doesServerSupportUnstableFeature: jest.fn().mockImplementation((feature) => {
+                    return feature === "org.matrix.msc2285.stable";
+                }),
+            } as any)).toBe(ReceiptType.ReadPrivate);
+        });
+
+        it('should return m.read.private if server supports stable and unstable', async () => {
+            expect(await utils.getPrivateReadReceiptField({
+                doesServerSupportUnstableFeature: jest.fn().mockImplementation((feature) => {
+                    return ["org.matrix.msc2285.stable", "org.matrix.msc2285"].includes(feature);
+                }),
+            } as any)).toBe(ReceiptType.ReadPrivate);
+        });
+
+        it('should return org.matrix.msc2285.read.private if server supports  unstable', async () => {
+            expect(await utils.getPrivateReadReceiptField({
+                doesServerSupportUnstableFeature: jest.fn().mockImplementation((feature) => {
+                    return feature === "org.matrix.msc2285";
+                }),
+            } as any)).toBe(ReceiptType.UnstableReadPrivate);
+        });
+
+        it('should return none if server does not support either', async () => {
+            expect(await utils.getPrivateReadReceiptField({
+                doesServerSupportUnstableFeature: jest.fn().mockResolvedValue(false),
+            } as any)).toBeFalsy();
+        });
+    });
+
+    describe('isSupportedReceiptType', () => {
+        it('should support m.read', () => {
+            expect(utils.isSupportedReceiptType(ReceiptType.Read)).toBeTruthy();
+        });
+
+        it('should support m.read.private', () => {
+            expect(utils.isSupportedReceiptType(ReceiptType.ReadPrivate)).toBeTruthy();
+        });
+
+        it('should support org.matrix.msc2285.read.private', () => {
+            expect(utils.isSupportedReceiptType(ReceiptType.UnstableReadPrivate)).toBeTruthy();
+        });
+
+        it('should not support other receipt types', () => {
+            expect(utils.isSupportedReceiptType("this is a receipt type")).toBeFalsy();
+        });
+    });
 });
diff --git a/src/@types/read_receipts.ts b/src/@types/read_receipts.ts
@@ -17,5 +17,9 @@ limitations under the License.
 export enum ReceiptType {
     Read = "m.read",
     FullyRead = "m.fully_read",
-    ReadPrivate = "org.matrix.msc2285.read.private"
+    ReadPrivate = "m.read.private",
+    /**
+     * @deprecated Please use the ReadPrivate type when possible. This value may be removed at any time without notice.
+     */
+    UnstableReadPrivate = "org.matrix.msc2285.read.private",
 }
diff --git a/src/client.ts b/src/client.ts
@@ -40,7 +40,6 @@ import { sleep } from './utils';
 import { Direction, EventTimeline } from "./models/event-timeline";
 import { IActionsObject, PushProcessor } from "./pushprocessor";
 import { AutoDiscovery, AutoDiscoveryAction } from "./autodiscovery";
-import { IEncryptAndSendToDevicesResult } from "./crypto";
 import * as olmlib from "./crypto/olmlib";
 import { decodeBase64, encodeBase64 } from "./crypto/olmlib";
 import { IExportedDevice as IExportedOlmDevice } from "./crypto/OlmDevice";
@@ -1089,11 +1088,12 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
                 // Figure out if we've read something or if it's just informational
                 const content = event.getContent();
                 const isSelf = Object.keys(content).filter(eid => {
-                    const read = content[eid][ReceiptType.Read];
-                    if (read && Object.keys(read).includes(this.getUserId())) return true;
+                    for (const [key, value] of Object.entries(content[eid])) {
+                        if (!utils.isSupportedReceiptType(key)) continue;
+                        if (!value) continue;
 
-                    const readPrivate = content[eid][ReceiptType.ReadPrivate];
-                    if (readPrivate && Object.keys(readPrivate).includes(this.getUserId())) return true;
+                        if (Object.keys(value).includes(this.getUserId())) return true;
+                    }
 
                     return false;
                 }).length > 0;
@@ -2583,7 +2583,7 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
     public encryptAndSendToDevices(
         userDeviceInfoArr: IOlmDevice<DeviceInfo>[],
         payload: object,
-    ): Promise<IEncryptAndSendToDevicesResult> {
+    ): Promise<void> {
         if (!this.crypto) {
             throw new Error("End-to-End encryption disabled");
         }
@@ -4661,7 +4661,7 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
             room?.addLocalEchoReceipt(this.credentials.userId, rpEvent, ReceiptType.ReadPrivate);
         }
 
-        return this.setRoomReadMarkersHttpRequest(roomId, rmEventId, rrEventId, rpEventId);
+        return await this.setRoomReadMarkersHttpRequest(roomId, rmEventId, rrEventId, rpEventId);
     }
 
     /**
@@ -7501,7 +7501,7 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
      * don't want other users to see the read receipts. This is experimental. Optional.
      * @return {Promise} Resolves: the empty object, {}.
      */
-    public setRoomReadMarkersHttpRequest(
+    public async setRoomReadMarkersHttpRequest(
         roomId: string,
         rmEventId: string,
         rrEventId: string,
@@ -7514,9 +7514,13 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
         const content = {
             [ReceiptType.FullyRead]: rmEventId,
             [ReceiptType.Read]: rrEventId,
-            [ReceiptType.ReadPrivate]: rpEventId,
         };
 
+        const privateField = await utils.getPrivateReadReceiptField(this);
+        if (privateField) {
+            content[privateField] = rpEventId;
+        }
+
         return this.http.authedRequest(undefined, Method.Post, path, undefined, content);
     }
 
diff --git a/src/crypto/algorithms/megolm.ts b/src/crypto/algorithms/megolm.ts
@@ -606,19 +606,19 @@ class MegolmEncryption extends EncryptionAlgorithm {
     private encryptAndSendKeysToDevices(
         session: OutboundSessionInfo,
         chainIndex: number,
-        userDeviceMap: IOlmDevice[],
+        devices: IOlmDevice[],
         payload: IPayload,
     ): Promise<void> {
         return this.crypto.encryptAndSendToDevices(
-            userDeviceMap,
+            devices,
             payload,
-        ).then(({ toDeviceBatch, deviceInfoByUserIdAndDeviceId }) => {
+        ).then(() => {
             // store that we successfully uploaded the keys of the current slice
-            for (const msg of toDeviceBatch.batch) {
+            for (const device of devices) {
                 session.markSharedWithDevice(
-                    msg.userId,
-                    msg.deviceId,
-                    deviceInfoByUserIdAndDeviceId.get(msg.userId).get(msg.deviceId).getIdentityKey(),
+                    device.userId,
+                    device.deviceInfo.deviceId,
+                    device.deviceInfo.getIdentityKey(),
                     chainIndex,
                 );
             }
diff --git a/src/crypto/index.ts b/src/crypto/index.ts
@@ -212,11 +212,6 @@ export interface IEncryptedContent {
 }
 /* eslint-enable camelcase */
 
-export interface IEncryptAndSendToDevicesResult {
-    toDeviceBatch: ToDeviceBatch;
-    deviceInfoByUserIdAndDeviceId: Map<string, Map<string, DeviceInfo>>;
-}
-
 export enum CryptoEvent {
     DeviceVerificationChanged = "deviceVerificationChanged",
     UserTrustStatusChanged = "userTrustStatusChanged",
@@ -3129,12 +3124,11 @@ export class Crypto extends TypedEventEmitter<CryptoEvent, CryptoEventHandlerMap
     public async encryptAndSendToDevices(
         userDeviceInfoArr: IOlmDevice<DeviceInfo>[],
         payload: object,
-    ): Promise<IEncryptAndSendToDevicesResult> {
+    ): Promise<void> {
         const toDeviceBatch: ToDeviceBatch = {
             eventType: EventType.RoomMessageEncrypted,
             batch: [],
         };
-        const deviceInfoByUserIdAndDeviceId = new Map<string, Map<string, DeviceInfo>>();
 
         try {
             await Promise.all(userDeviceInfoArr.map(async ({ userId, deviceInfo }) => {
@@ -3145,17 +3139,6 @@ export class Crypto extends TypedEventEmitter<CryptoEvent, CryptoEventHandlerMap
                     ciphertext: {},
                 };
 
-                // Assign to temp value to make type-checking happy
-                let userIdDeviceInfo = deviceInfoByUserIdAndDeviceId.get(userId);
-
-                if (userIdDeviceInfo === undefined) {
-                    userIdDeviceInfo = new Map<string, DeviceInfo>();
-                    deviceInfoByUserIdAndDeviceId.set(userId, userIdDeviceInfo);
-                }
-
-                // We hold by reference, this updates deviceInfoByUserIdAndDeviceId[userId]
-                userIdDeviceInfo.set(deviceId, deviceInfo);
-
                 toDeviceBatch.batch.push({
                     userId,
                     deviceId,
@@ -3193,7 +3176,6 @@ export class Crypto extends TypedEventEmitter<CryptoEvent, CryptoEventHandlerMap
 
             try {
                 await this.baseApis.queueToDevice(toDeviceBatch);
-                return { toDeviceBatch, deviceInfoByUserIdAndDeviceId };
             } catch (e) {
                 logger.error("sendToDevice failed", e);
                 throw e;
diff --git a/src/models/room.ts b/src/models/room.ts
diff --git a/src/store/indexeddb-local-backend.ts b/src/store/indexeddb-local-backend.ts
diff --git a/src/sync-accumulator.ts b/src/sync-accumulator.ts
diff --git a/src/utils.ts b/src/utils.ts
