Fix a case where joinRoom creates a duplicate Room object (#3747)

When calling MatrixClient.joinRoom with a room alias, the method would create a new Room object, even if you were already present in that room. This changes its behavior to no-op, as the doc comment promises.
matrix-org · Sep 25, 2023 · 8f90159 · 8f90159
1 parent d29b852
commit 8f90159
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 5 deletions.
diff --git a/spec/integ/matrix-client-methods.spec.ts b/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 () => {

diff --git a/src/client.ts b/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")) {
-            return Promise.resolve(room);
-        }
+        if (room?.hasMembershipState(this.credentials.userId!, "join")) return 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) {