Merge branch 'develop' into no_re_backup

.github/workflows/cypress.yml

@@ -70,13 +70,21 @@ jobs:
                   authToken: "${{ secrets.GITHUB_TOKEN }}"
                   state: success
                   description: Cypress skipped
-                  context: "${{ github.workflow }} / Cypress"
+
+                  # Keep in step with the `context` that is updated by `Sibz/github-status-action`
+                  # in matrix-org/matrix-react-sdk/.github/workflows/cypress.yaml.
+                  context: "${{ github.workflow }} / cypress"
+
                   sha: "${{ github.event.workflow_run.head_sha }}"
 
             - uses: Sibz/github-status-action@071b5370da85afbb16637d6eed8524a06bc2053e # v1
               with:
                   authToken: "${{ secrets.GITHUB_TOKEN }}"
                   state: success
                   description: Playwright skipped
-                  context: "${{ github.workflow }} / Playwright"
+
+                  # Keep in step with the `context` that is updated by `Sibz/github-status-action`
+                  # in matrix-org/matrix-react-sdk/.github/workflows/end-to-end-tests.yaml.
+                  context: "${{ github.workflow }} / end-to-end-tests"
+
                   sha: "${{ github.event.workflow_run.head_sha }}"

spec/integ/crypto/crypto.spec.ts

@@ -2832,6 +2832,19 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string,
 
                 const newBackupUploadPromise = awaitMegolmBackupKeyUpload();
 
+                // Track calls to scheduleAllGroupSessionsForBackup. This is
+                // only relevant on legacy encryption.
+                const scheduleAllGroupSessionsForBackup = jest.fn();
+                if (backend === "libolm") {
+                    aliceClient.crypto!.backupManager.scheduleAllGroupSessionsForBackup =
+                        scheduleAllGroupSessionsForBackup;
+                } else {
+                    // With Rust crypto, we don't need to call this function, so
+                    // we call the dummy value here so we pass our later
+                    // expectation.
+                    scheduleAllGroupSessionsForBackup();
+                }
+
                 await aliceClient.getCrypto()!.resetKeyBackup();
                 await awaitDeleteCalled;
                 await newBackupStatusUpdate;
@@ -2843,6 +2856,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("crypto (%s)", (backend: string,
                 expect(nextVersion).toBeDefined();
                 expect(nextVersion).not.toEqual(currentVersion);
                 expect(nextKey).not.toEqual(currentBackupKey);
+                expect(scheduleAllGroupSessionsForBackup).toHaveBeenCalled();
 
                 // The `deleteKeyBackupVersion` API is deprecated but has been modified to work with both crypto backend
                 // ensure that it works anyhow

spec/integ/matrix-client-unread-notifications.spec.ts

@@ -28,31 +28,69 @@ import {
     NotificationCountType,
     RelationType,
     Room,
+    fixNotificationCountOnDecryption,
 } from "../../src";
 import { TestClient } from "../TestClient";
 import { ReceiptType } from "../../src/@types/read_receipts";
 import { mkThread } from "../test-utils/thread";
 import { SyncState } from "../../src/sync";
 
-describe("MatrixClient syncing", () => {
-    const userA = "@alice:localhost";
-    const userB = "@bob:localhost";
+const userA = "@alice:localhost";
+const userB = "@bob:localhost";
+const selfUserId = userA;
+const selfAccessToken = "aseukfgwef";
 
-    const selfUserId = userA;
-    const selfAccessToken = "aseukfgwef";
+function setupTestClient(): [MatrixClient, HttpBackend] {
+    const testClient = new TestClient(selfUserId, "DEVICE", selfAccessToken);
+    const httpBackend = testClient.httpBackend;
+    const client = testClient.client;
+    httpBackend!.when("GET", "/versions").respond(200, {});
+    httpBackend!.when("GET", "/pushrules").respond(200, {});
+    httpBackend!.when("POST", "/filter").respond(200, { filter_id: "a filter id" });
+    return [client, httpBackend];
+}
 
+describe("Notification count fixing", () => {
     let client: MatrixClient | undefined;
-    let httpBackend: HttpBackend | undefined;
 
-    const setupTestClient = (): [MatrixClient, HttpBackend] => {
-        const testClient = new TestClient(selfUserId, "DEVICE", selfAccessToken);
-        const httpBackend = testClient.httpBackend;
-        const client = testClient.client;
-        httpBackend!.when("GET", "/versions").respond(200, {});
-        httpBackend!.when("GET", "/pushrules").respond(200, {});
-        httpBackend!.when("POST", "/filter").respond(200, { filter_id: "a filter id" });
-        return [client, httpBackend];
-    };
+    beforeEach(() => {
+        [client] = setupTestClient();
+    });
+
+    it("doesn't increment notification count for events that can't be found in a room", async () => {
+        const roomId = "!room:localhost";
+
+        client!.startClient({ threadSupport: true });
+        const room = new Room(roomId, client!, selfUserId);
+        jest.spyOn(client!, "getRoom").mockImplementation((id) => (id === roomId ? room : null));
+
+        const event = new MatrixEvent({
+            room_id: roomId,
+            type: "m.reaction",
+            event_id: "$foo",
+            content: {
+                "m.relates_to": {
+                    rel_type: RelationType.Annotation,
+                    event_id: "$foo",
+                    key: "x",
+                },
+            },
+        });
+
+        jest.spyOn(event, "getPushActions").mockReturnValue({
+            notify: true,
+            tweaks: {},
+        });
+
+        fixNotificationCountOnDecryption(client!, event);
+
+        expect(room.getUnreadNotificationCount(NotificationCountType.Total)).toBe(0);
+    });
+});
+
+describe("MatrixClient syncing", () => {
+    let client: MatrixClient | undefined;
+    let httpBackend: HttpBackend | undefined;
 
     beforeEach(() => {
         [client, httpBackend] = setupTestClient();

spec/unit/notifications.spec.ts

@@ -106,6 +106,8 @@ describe("fixNotificationCountOnDecryption", () => {
             mockClient,
         );
 
+        room.addLiveEvents([event]);
+
         THREAD_ID = event.getId()!;
         threadEvent = mkEvent({
             type: EventType.RoomMessage,

src/client.ts

@@ -3646,6 +3646,9 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
     /**
      * Marks all group sessions as needing to be backed up and schedules them to
      * upload in the background as soon as possible.
+     *
+     * (This is done automatically as part of {@link CryptoApi.resetKeyBackup},
+     * so there is probably no need to call this manually.)
      */
     public async scheduleAllGroupSessionsForBackup(): Promise<void> {
         if (!this.crypto) {
@@ -3658,6 +3661,10 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
     /**
      * Marks all group sessions as needing to be backed up without scheduling
      * them to upload in the background.
+     *
+     * (This is done automatically as part of {@link CryptoApi.resetKeyBackup},
+     * so there is probably no need to call this manually.)
+     *
      * @returns Promise which resolves to the number of sessions requiring a backup.
      */
     public flagAllGroupSessionsForBackup(): Promise<number> {
@@ -9837,6 +9844,19 @@ export function fixNotificationCountOnDecryption(cli: MatrixClient, event: Matri
     const room = cli.getRoom(event.getRoomId());
     if (!room || !ourUserId || !eventId) return;
 
+    // Due to threads, we can get relation events (eg. edits & reactions) that never get
+    // added to a timeline and so cannot be found in their own room (their edit / reaction
+    // still applies to the event it needs to, so it doesn't matter too much). However, if
+    // we try to process notification about this event, we'll get very confused because we
+    // won't be able to find the event in the room, so will assume it must be unread, even
+    // if it's actually read. We therefore skip anything that isn't in the room. This isn't
+    // *great*, so if we can fix the homeless events (eg. with MSC4023) then we should probably
+    // remove this workaround.
+    if (!room.findEventById(eventId)) {
+        logger.info("Decrypted event is not in the room: ignoring");
+        return;
+    }
+
     const isThreadEvent = !!event.threadRootId && !event.isThreadRoot;
 
     let hasReadEvent;

src/crypto/index.ts

@@ -1211,6 +1211,7 @@ export class Crypto extends TypedEventEmitter<CryptoEvent, CryptoEventHandlerMap
         await this.storeSessionBackupPrivateKey(privateKey);
 
         await this.backupManager.checkAndStart();
+        await this.backupManager.scheduleAllGroupSessionsForBackup();
     }
 
     /**

src/http-api/index.ts

@@ -50,7 +50,7 @@ export class MatrixHttpApi<O extends IHttpOpts> extends FetchHttpApi<O> {
         const abortController = opts.abortController ?? new AbortController();
 
         // If the file doesn't have a mime type, use a default since the HS errors if we don't supply one.
-        const contentType = opts.type ?? (file as File).type ?? "application/octet-stream";
+        const contentType = (opts.type ?? (file as File).type) || "application/octet-stream";
         const fileName = opts.name ?? (file as File).name;
 
         const upload = {

src/models/room-receipts.ts

@@ -18,6 +18,7 @@ import { MAIN_ROOM_TIMELINE, Receipt, ReceiptContent } from "../@types/read_rece
 import { threadIdForReceipt } from "../client";
 import { Room, RoomEvent } from "./room";
 import { MatrixEvent } from "./event";
+import { logger } from "../logger";
 
 /**
  * The latest receipts we have for a room.
@@ -140,6 +141,11 @@ export class RoomReceipts {
         const event = this.room.findEventById(eventId);
         if (!event) {
             // We don't know whether the user has read it - default to caution and say no.
+            // This shouldn't really happen and feels like it ought to be an exception: let's
+            // log a warn for now.
+            logger.warn(
+                `hasUserReadEvent event ID ${eventId} not found in room ${this.room.roomId}: this shouldn't happen!`,
+            );
             return false;
         }