Merge branch 'develop' into valere/element-r/backup/restore_test

.github/workflows/cypress-skipped.yml

@@ -1,29 +0,0 @@
-# We only want to run the cypress tests in the merge queue (to save CI time),
-# but we do want to make it a required check for the merge queue.
-#
-# Unfortunately, github doesn't distinguish between "checks needed for branch
-# protection" (ie, the things that must pass before the PR will even be added
-# to the merge queue) and "checks needed in the merge queue". We just have to add
-# the check to the branch protection list.
-#
-# Ergo, if we know we're not going to run the cypress tests, we need to add a
-# passing status check manually.
-
-name: Mark cypress skipped
-on:
-    pull_request: {}
-permissions:
-    statuses: write
-jobs:
-    mark_skipped:
-        runs-on: ubuntu-latest
-        steps:
-            - uses: Sibz/github-status-action@650dd1a882a76dbbbc4576fb5974b8d22f29847f # v1.1.6
-              with:
-                  authToken: "${{ secrets.GITHUB_TOKEN }}"
-                  state: success
-                  description: Cypress skipped
-                  context: "matrix-react-sdk Cypress End to End Tests / cypress"
-                  sha: "${{ github.event.pull_request.head.sha }}"
-                  # link to this file
-                  target_url: "${{ github.event.pull_request.head.repo.html_url }}/blob/${{ github.event.pull_request.head.ref }}/.github/workflows/cypress-skipped.yml"

.github/workflows/cypress.yml

@@ -15,6 +15,11 @@ concurrency:
 jobs:
     cypress:
         name: Cypress
+
+        # We only want to run the cypress tests on merge queue to prevent regressions
+        # from creeping in. They take a long time to run and consume 4 concurrent runners.
+        if: github.event.workflow_run.event == 'merge_group'
+
         uses: matrix-org/matrix-react-sdk/.github/workflows/cypress.yaml@v3.80.1
         permissions:
             actions: read
@@ -29,3 +34,26 @@ jobs:
         with:
             react-sdk-repository: matrix-org/matrix-react-sdk
             rust-crypto: true
+
+    # We want to make the cypress tests a required check for the merge queue.
+    #
+    # Unfortunately, github doesn't distinguish between "checks needed for branch
+    # protection" (ie, the things that must pass before the PR will even be added
+    # to the merge queue) and "checks needed in the merge queue". We just have to add
+    # the check to the branch protection list.
+    #
+    # Ergo, if we know we're not going to run the cypress tests, we need to add a
+    # passing status check manually.
+    mark_skipped:
+        if: github.event.workflow_run.event != 'merge_group'
+        permissions:
+            statuses: write
+        runs-on: ubuntu-latest
+        steps:
+            - uses: Sibz/github-status-action@650dd1a882a76dbbbc4576fb5974b8d22f29847f # v1.1.6
+              with:
+                  authToken: "${{ secrets.GITHUB_TOKEN }}"
+                  state: success
+                  description: Cypress skipped
+                  context: "${{ github.workflow }} / cypress"
+                  sha: "${{ github.event.workflow_run.head_sha }}"

.github/workflows/downstream-artifacts.yml

@@ -1,11 +1,10 @@
 name: Build downstream artifacts
 on:
-    # We only want the Rust Crypto Cypress tests on merge queue to prevent regressions
-    # from creeping in. They take a long time to run and consume 4 concurrent runners.
-    # Anyone working on Rust Crypto is able to run the tests locally if required.
     merge_group:
         types: [checks_requested]
 
+    pull_request: {}
+
     # For now at least, we don't run this or the cypress-tests against pushes
     # to develop or master.
     #

spec/integ/matrix-client-methods.spec.ts

@@ -150,7 +150,7 @@ describe("MatrixClient", function () {
     });
 
     describe("joinRoom", function () {
-        it("should no-op if you've already joined a room", function () {
+        it("should no-op given the ID of a room you've already joined", async () => {
             const roomId = "!foo:bar";
             const room = new Room(roomId, client, userId);
             client.fetchRoomEvent = () =>
@@ -168,8 +168,32 @@ describe("MatrixClient", function () {
             ]);
             httpBackend.verifyNoOutstandingRequests();
             store.storeRoom(room);
-            client.joinRoom(roomId);
+
+            const joinPromise = client.joinRoom(roomId);
             httpBackend.verifyNoOutstandingRequests();
+            expect(await joinPromise).toBe(room);
+        });
+
+        it("should no-op given the alias of a room you've already joined", async () => {
+            const roomId = "!roomId:server";
+            const roomAlias = "#my-fancy-room:server";
+            const room = new Room(roomId, client, userId);
+            room.addLiveEvents([
+                utils.mkMembership({
+                    user: userId,
+                    room: roomId,
+                    mship: "join",
+                    event: true,
+                }),
+            ]);
+            store.storeRoom(room);
+
+            // The method makes a request to resolve the alias
+            httpBackend.when("POST", "/join/" + encodeURIComponent(roomAlias)).respond(200, { room_id: roomId });
+
+            const joinPromise = client.joinRoom(roomAlias);
+            await httpBackend.flushAllExpected();
+            expect(await joinPromise).toBe(room);
         });
 
         it("should send request to inviteSignUrl if specified", async () => {

spec/unit/matrixrtc/CallMembership.spec.ts

@@ -23,6 +23,7 @@ const membershipTemplate: CallMembershipData = {
     application: "m.call",
     device_id: "AAAAAAA",
     expires: 5000,
+    membershipID: "bloop",
 };
 
 function makeMockEvent(originTs = 0): MatrixEvent {

spec/unit/matrixrtc/MatrixRTCSession.spec.ts

@@ -26,6 +26,7 @@ const membershipTemplate: CallMembershipData = {
     application: "m.call",
     device_id: "AAAAAAA",
     expires: 60 * 60 * 1000,
+    membershipID: "bloop",
 };
 
 const mockFocus = { type: "mock" };
@@ -56,6 +57,7 @@ describe("MatrixRTCSession", () => {
         expect(sess?.memberships[0].scope).toEqual("m.room");
         expect(sess?.memberships[0].application).toEqual("m.call");
         expect(sess?.memberships[0].deviceId).toEqual("AAAAAAA");
+        expect(sess?.memberships[0].membershipID).toEqual("bloop");
         expect(sess?.memberships[0].isExpired()).toEqual(false);
     });
 
@@ -219,6 +221,7 @@ describe("MatrixRTCSession", () => {
                             device_id: "AAAAAAA",
                             expires: 3600000,
                             foci_active: [{ type: "mock" }],
+                            membershipID: expect.stringMatching(".*"),
                         },
                     ],
                 },
@@ -286,6 +289,7 @@ describe("MatrixRTCSession", () => {
                                 expires: 3600000 * 2,
                                 foci_active: [{ type: "mock" }],
                                 created_ts: 1000,
+                                membershipID: expect.stringMatching(".*"),
                             },
                         ],
                     },
@@ -357,6 +361,7 @@ describe("MatrixRTCSession", () => {
                         device_id: "AAAAAAA",
                         expires: 3600000,
                         foci_active: [mockFocus],
+                        membershipID: expect.stringMatching(".*"),
                     },
                 ],
             },
@@ -388,6 +393,7 @@ describe("MatrixRTCSession", () => {
                         device_id: "OTHERDEVICE",
                         expires: 3600000,
                         created_ts: 1000,
+                        membershipID: expect.stringMatching(".*"),
                     },
                     {
                         application: "m.call",
@@ -396,6 +402,7 @@ describe("MatrixRTCSession", () => {
                         device_id: "AAAAAAA",
                         expires: 3600000,
                         foci_active: [mockFocus],
+                        membershipID: expect.stringMatching(".*"),
                     },
                 ],
             },

spec/unit/matrixrtc/MatrixRTCSessionManager.spec.ts

@@ -26,6 +26,7 @@ const membershipTemplate: CallMembershipData = {
     application: "m.call",
     device_id: "AAAAAAA",
     expires: 60 * 60 * 1000,
+    membershipID: "bloop",
 };
 
 describe("MatrixRTCSessionManager", () => {

src/client.ts

@@ -4187,9 +4187,7 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
         }
 
         const room = this.getRoom(roomIdOrAlias);
-        if (room?.hasMembershipState(this.credentials.userId!, "join")) {
+        if (room?.hasMembershipState(this.credentials.userId!, "join")) return room;
-            return Promise.resolve(room);
-        }
 
         let signPromise: Promise<IThirdPartySigned | void> = Promise.resolve();
 
@@ -4214,6 +4212,12 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
         const res = await this.http.authedRequest<{ room_id: string }>(Method.Post, path, queryString, data);
 
         const roomId = res.room_id;
+        // In case we were originally given an alias, check the room cache again
+        // with the resolved ID - this method is supposed to no-op if we already
+        // were in the room, after all.
+        const resolvedRoom = this.getRoom(roomId);
+        if (resolvedRoom?.hasMembershipState(this.credentials.userId!, "join")) return resolvedRoom;
+
         const syncApi = new SyncApi(this, this.clientOpts, this.buildSyncApiOptions());
         const syncRoom = syncApi.createRoom(roomId);
         if (opts.syncRoom) {

src/matrixrtc/CallMembership.ts

@@ -29,6 +29,7 @@ export interface CallMembershipData {
     created_ts?: number;
     expires: number;
     foci_active?: Focus[];
+    membershipID: string;
 }
 
 export class CallMembership {
@@ -64,6 +65,10 @@ export class CallMembership {
         return this.data.scope;
     }
 
+    public get membershipID(): string {
+        return this.data.membershipID;
+    }
+
     public createdTs(): number {
         return this.data.created_ts ?? this.parentEvent.getTs();
     }

src/matrixrtc/MatrixRTCSession.ts

@@ -23,6 +23,7 @@ import { EventType } from "../@types/event";
 import { CallMembership, CallMembershipData } from "./CallMembership";
 import { Focus } from "./focus";
 import { MatrixEvent } from "../matrix";
+import { randomString } from "../randomstring";
 
 const MEMBERSHIP_EXPIRY_TIME = 60 * 60 * 1000;
 const MEMBER_EVENT_CHECK_PERIOD = 2 * 60 * 1000; // How often we check to see if we need to re-send our member event
@@ -54,6 +55,14 @@ export class MatrixRTCSession extends TypedEventEmitter<MatrixRTCSessionEvent, M
     // if we're not yet joined
     private relativeExpiry: number | undefined;
 
+    // An identifier for our membership of the call. This will allow us to easily recognise
+    // whether a membership was sent by this session or is stale from some other time.
+    // It also forces our membership events to be unique, because otherwise we could try
+    // to overwrite a membership from a previous session but it would do nothing because the
+    // event content would be identical. We need the origin_server_ts to update though, so
+    // forcing unique content fixes this.
+    private membershipId: string | undefined;
+
     private memberEventTimeout?: ReturnType<typeof setTimeout>;
     private expiryTimeout?: ReturnType<typeof setTimeout>;
 
@@ -174,6 +183,7 @@ export class MatrixRTCSession extends TypedEventEmitter<MatrixRTCSessionEvent, M
         logger.info(`Joining call session in room ${this.room.roomId}`);
         this.activeFoci = activeFoci;
         this.relativeExpiry = MEMBERSHIP_EXPIRY_TIME;
+        this.membershipId = randomString(5);
         this.emit(MatrixRTCSessionEvent.JoinStateChanged, true);
         // We don't wait for this, mostly because it may fail and schedule a retry, so this
         // function returning doesn't really mean anything at all.
@@ -195,6 +205,7 @@ export class MatrixRTCSession extends TypedEventEmitter<MatrixRTCSessionEvent, M
         logger.info(`Leaving call session in room ${this.room.roomId}`);
         this.relativeExpiry = undefined;
         this.activeFoci = undefined;
+        this.membershipId = undefined;
         this.emit(MatrixRTCSessionEvent.JoinStateChanged, false);
         this.triggerCallMembershipEventUpdate();
     }
@@ -249,6 +260,9 @@ export class MatrixRTCSession extends TypedEventEmitter<MatrixRTCSessionEvent, M
         if (this.relativeExpiry === undefined) {
             throw new Error("Tried to create our own membership event when we're not joined!");
         }
+        if (this.membershipId === undefined) {
+            throw new Error("Tried to create our own membership event when we have no membership ID!");
+        }
 
         const m: CallMembershipData = {
             call_id: "",
@@ -257,6 +271,7 @@ export class MatrixRTCSession extends TypedEventEmitter<MatrixRTCSessionEvent, M
             device_id: this.client.getDeviceId()!,
             expires: this.relativeExpiry,
             foci_active: this.activeFoci,
+            membershipID: this.membershipId,
         };
 
         if (prevMembership) m.created_ts = prevMembership.createdTs();
@@ -373,7 +388,7 @@ export class MatrixRTCSession extends TypedEventEmitter<MatrixRTCSessionEvent, M
         const myPrevMembershipData = memberships.find((m) => m.device_id === localDeviceId);
         let myPrevMembership;
         try {
-            if (myCallMemberEvent && myPrevMembershipData) {
+            if (myCallMemberEvent && myPrevMembershipData && myPrevMembershipData.membershipID === this.membershipId) {
                 myPrevMembership = new CallMembership(myCallMemberEvent, myPrevMembershipData);
             }
         } catch (e) {