Merge pull request #241 from matrix-org/hs/join-room_id

Return roomId on join
matrix-org · Oct 5, 2020 · 332442b · 332442b
2 parents 7d1544d + efd1bb6
commit 332442b
Show file tree

Hide file tree

Showing 6 changed files with 56 additions and 38 deletions.
diff --git a/changelog.d/241.feature b/changelog.d/241.feature
@@ -0,0 +1 @@
+Return the roomId when calling intent.join
diff --git a/spec/unit/intent.spec.js b/spec/unit/intent.spec.js
@@ -1,5 +1,4 @@
-"use strict";
-const Intent = require("../..").Intent;
+const { Intent } = require("../..");
 const log = require("../log");
 
 describe("Intent", function() {
@@ -31,11 +30,12 @@ describe("Intent", function() {
 
         it("should /join/$ROOMID if it doesn't know it is already joined",
         function() {
-            client.joinRoom.and.callFake(() => Promise.resolve({}));
-            return intent.join(roomId).then(function() {
+            client.joinRoom.and.callFake(() => Promise.resolve({room_id: roomId}));
+            return intent.join(roomId).then(function(resultRoomId) {
                 expect(client.joinRoom).toHaveBeenCalledWith(
                     roomId, { syncRoom: false }
                 );
+                expect(resultRoomId).toBe(roomId);
             });
         });
 
@@ -49,7 +49,8 @@ describe("Intent", function() {
                     membership: "join"
                 }
             });
-            return intent.join(roomId).then(function() {
+            return intent.join(roomId).then(function(resultRoomId) {
+                expect(resultRoomId).toBe(roomId);
                 expect(client.joinRoom).not.toHaveBeenCalled();
             });
         });
@@ -75,15 +76,16 @@ describe("Intent", function() {
                             error: "Join first"
                         });
                     }
-                    return Promise.resolve({});
+                    return Promise.resolve({room_id: roomId});
                 });
                 botClient.invite.and.callFake(() => Promise.resolve({}));
 
-                return intent.join(roomId).then(function() {
+                return intent.join(roomId).then(function(resultRoomId) {
                     expect(client.joinRoom).toHaveBeenCalledWith(
                         roomId, { syncRoom: false }
                     );
                     expect(botClient.invite).toHaveBeenCalledWith(roomId, userId);
+                    expect(resultRoomId).toBe(roomId);
                 });
             });
 
@@ -108,16 +110,18 @@ describe("Intent", function() {
                         }
                         return Promise.resolve({});
                     });
-                    botClient.joinRoom.and.callFake(() => Promise.resolve({}));
+                    botClient.joinRoom.and.callFake(() => Promise.resolve({room_id: roomId}));
 
-                    return intent.join(roomId).then(function() {
+                    return intent.join(roomId).then(function(resultRoomId) {
                         expect(client.joinRoom).toHaveBeenCalledWith(
                             roomId, { syncRoom: false }
                         );
                         expect(botClient.invite).toHaveBeenCalledWith(roomId, userId);
                         expect(botClient.joinRoom).toHaveBeenCalledWith(
                             roomId, { syncRoom: false }
                         );
+                        expect(resultRoomId).toBe(roomId);
+
                     });
                 });
 
@@ -325,11 +329,12 @@ describe("Intent", function() {
                     room_id: joinRoomId,
                 });
             });
-            return intent.sendMessage(roomId, content).then(function() {
+            return intent.sendMessage(roomId, content).then(function(eventId) {
                 expect(client.sendEvent).toHaveBeenCalledWith(
                     roomId, "m.room.message", content
                 );
                 expect(client.joinRoom).toHaveBeenCalledWith(roomId, { syncRoom: false });
+                expect(eventId).toEqual({event_id: "$12345:6789"});
             });
         });
 

diff --git a/src/components/client-factory.ts b/src/components/client-factory.ts
@@ -14,6 +14,8 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
+import { Request } from "./request";
+
 type LogWrapCallback = (err: Error, response: { statusCode: number }, body: Record<string, unknown>) => void;
 type OriginalRequest = (opts: Record<string, unknown>, cb: LogWrapCallback) => void;
 
@@ -119,8 +121,7 @@ export class ClientFactory {
      * This factory will dispose the created client instance when the request is
      * resolved.
      */
-    // eslint-disable-next-line @typescript-eslint/no-explicit-any
-    public getClientAs(userId?: string, request?: any, urlOverride?: string, usingE2E = false) {
+    public getClientAs(userId?: string, request?: Request<unknown>, urlOverride?: string, usingE2E = false) {
         const reqId = request ? request.getId() : "-";
         const userIdKey = userId || "bot";
 

diff --git a/src/components/intent.ts b/src/components/intent.ts
@@ -516,12 +516,12 @@ export class Intent {
      * <p>Join a room</p>
      * This will automatically send an invite from the bot if it is an invite-only
      * room, which may make the bot attempt to join the room if it isn't already.
-     * @param roomId The room to join.
+     * @param roomIdOrAlias The room ID or room alias to join.
      * @param viaServers The server names to try and join through in
      * addition to those that are automatically chosen.
      */
-    public async join(roomId: string, viaServers?: string[]) {
-        await this._ensureJoined(roomId, false, viaServers);
+    public async join(roomIdOrAlias: string, viaServers?: string[]): Promise<string> {
+        return this._ensureJoined(roomIdOrAlias, false, viaServers);
     }
 
     /**
@@ -698,17 +698,18 @@ export class Intent {
     }
 
     private async _ensureJoined(
-        roomId: string, ignoreCache = false, viaServers?: string[], passthroughError = false
-    ) {
+        roomIdOrAlias: string, ignoreCache = false, viaServers?: string[], passthroughError = false
+    ): Promise<string> {
+        const isRoomId = roomIdOrAlias.startsWith("!");
         const { userId } = this.client.credentials;
         const opts: { syncRoom: boolean, viaServers?: string[] } = {
             syncRoom: false,
         };
         if (viaServers) {
             opts.viaServers = viaServers;
         }
-        if (this.opts.backingStore.getMembership(roomId, userId) === "join" && !ignoreCache) {
-            return Promise.resolve();
+        if (isRoomId && this.opts.backingStore.getMembership(roomIdOrAlias, userId) === "join" && !ignoreCache) {
+            return roomIdOrAlias;
         }
 
         /* Logic:
@@ -728,12 +729,12 @@ export class Intent {
         FAIL (bot can't get into the room)
         */
 
-        const deferredPromise = defer();
+        const deferredPromise = defer<string>();
 
         const mark = (room: string, state: UserMembership) => {
             this.opts.backingStore.setMembership(room, userId, state);
             if (state === "join") {
-                deferredPromise.resolve();
+                deferredPromise.resolve(room);
             }
         }
 
@@ -746,25 +747,31 @@ export class Intent {
                 return deferredPromise.promise;
             }
             try {
-                await this.client.joinRoom(roomId, opts);
-                mark(roomId, "join");
+                // eslint-disable-next-line camelcase
+                const { room_id } = await this.client.joinRoom(roomIdOrAlias, opts);
+                mark(room_id, "join");
             }
             catch (ex) {
                 if (ex.errcode !== "M_FORBIDDEN") {
                     throw ex;
                 }
                 try {
+                    if (!isRoomId) {
+                        throw Error("Can't invite via an alias");
+                    }
                     // Try bot inviting client
-                    await this.botClient.invite(roomId, userId);
-                    await this.client.joinRoom(roomId, opts);
-                    mark(roomId, "join");
+                    await this.botClient.invite(roomIdOrAlias, userId);
+                    // eslint-disable-next-line camelcase
+                    const { room_id } = await this.client.joinRoom(roomIdOrAlias, opts);
+                    mark(room_id, "join");
                 }
                 catch (_ex) {
                     // Try bot joining
-                    await this.botClient.joinRoom(roomId, opts)
-                    await this.botClient.invite(roomId, userId);
-                    await this.client.joinRoom(roomId, opts);
-                    mark(roomId, "join");
+                    // eslint-disable-next-line camelcase
+                    const { room_id } = await this.botClient.joinRoom(roomIdOrAlias, opts)
+                    await this.botClient.invite(room_id, userId);
+                    await this.client.joinRoom(room_id, opts);
+                    mark(room_id, "join");
                 }
             }
         }

diff --git a/src/components/prometheusmetrics.ts b/src/components/prometheusmetrics.ts
@@ -17,6 +17,7 @@ import PromClient, { Registry } from "prom-client";
 import { AgeCounters } from "./agecounters";
 import JsSdk from "matrix-js-sdk";
 import { Request, Response } from "express";
+import { Bridge } from "..";
 type CollectorFunction = () => void;
 
 export interface BridgeGaugesCounts {
@@ -372,8 +373,7 @@ export class PrometheusMetrics {
      * containing Express app.
      * @param {Bridge} bridge The containing Bridge instance.
      */
-    // eslint-disable-next-line @typescript-eslint/no-explicit-any
-    public addAppServicePath(bridge: any) {
+    public addAppServicePath(bridge: Bridge) {
         bridge.addAppServicePath({
             method: "GET",
             path: "/metrics",

diff --git a/src/components/room-upgrade-handler.ts b/src/components/room-upgrade-handler.ts
@@ -15,8 +15,8 @@ limitations under the License.
 import logging from "./logging";
 import { MatrixRoom } from "../models/rooms/matrix";
 import { MatrixUser } from "../models/users/matrix";
-import { Intent } from "./intent";
-import { RoomBridgeStore, RoomBridgeStoreEntry } from "./room-bridge-store";
+import { RoomBridgeStoreEntry } from "./room-bridge-store";
+import { Bridge } from "..";
 
 const log = logging.get("RoomUpgradeHandler");
 
@@ -67,7 +67,7 @@ export class RoomUpgradeHandler {
      * @param {RoomUpgradeHandler~Options} opts
      * @param {Bridge} bridge The parent bridge.
      */
-    constructor(private readonly opts: RoomUpgradeHandlerOpts, private readonly bridge: any) {
+    constructor(private readonly opts: RoomUpgradeHandlerOpts, private readonly bridge: Bridge) {
         if (opts.migrateGhosts !== false) {
             opts.migrateGhosts = true;
         }
@@ -101,7 +101,7 @@ export class RoomUpgradeHandler {
     }
 
     private async joinNewRoom(newRoomId: string, joinVia: string[] = []) {
-        const intent = this.bridge.getIntent() as Intent;
+        const intent = this.bridge.getIntent();
         try {
             await intent.join(newRoomId, joinVia);
             return true;
@@ -168,7 +168,7 @@ export class RoomUpgradeHandler {
             const userIds = Object.keys(members).filter((u) => asBot.isRemoteUser(u));
             log.debug(`Migrating ${userIds.length} ghosts`);
             for (const userId of userIds) {
-                const i = this.bridge.getIntent(userId) as Intent;
+                const i = this.bridge.getIntent(userId);
                 await i.leave(oldRoomId);
                 await i.join(newRoomId);
             }
@@ -182,7 +182,11 @@ export class RoomUpgradeHandler {
     }
 
     private async migrateStoreEntries(oldRoomId: string, newRoomId: string) {
-        const roomStore = this.bridge.getRoomStore() as RoomBridgeStore;
+        const roomStore = this.bridge.getRoomStore();
+        if (!roomStore) {
+            // Do not migrate if we don't have a room store.
+            return true;
+        }
         const entries = await roomStore.getEntriesByMatrixId(oldRoomId);
         let success = false;
         // Upgrades are critical to get right, or a room will be stuck