Merge pull request #366 from Half-Shot/hs/fix-provisioner-not-waiting

Fix provisioner not waiting

src/provisioner.ts

@@ -10,7 +10,7 @@ const PERMISSION_REQUEST_TIMEOUT = 300000; // 5 minutes
 export class Provisioner {
 
     private bridge: Bridge;
-    private pendingRequests: { [channelId: string]: (approved: boolean) => void } = {}; // [channelId]: resolver fn
+    private pendingRequests: Map<string, (approved: boolean) => void> = new Map(); // [channelId]: resolver fn
 
     public SetBridge(bridge: Bridge): void {
         this.bridge = bridge;
@@ -32,38 +32,47 @@ export class Provisioner {
         return this.bridge.getRoomStore().removeEntriesByRemoteRoomId(remoteRoom.getId());
     }
 
-    public async AskBridgePermission(channel: Discord.TextChannel, requestor: string): Promise<void> {
+    public async AskBridgePermission(
+        channel: Discord.TextChannel,
+        requestor: string,
+        timeout: number = PERMISSION_REQUEST_TIMEOUT): Promise<string> {
         const channelId = `${channel.guild.id}/${channel.id}`;
 
         let responded = false;
+        let resolve: (msg: string) => void;
+        let reject: (err: Error) => void;
+        const deferP: Promise<string> = new Promise((res, rej) => {resolve = res; reject = rej; });
+
         const approveFn = (approved: boolean, expired = false) => {
             if (responded) {
                 return;
             }
 
             responded = true;
-            delete this.pendingRequests[channelId];
+            this.pendingRequests.delete(channelId);
             if (approved) {
-                return;
+                resolve("Approved");
             } else {
                 if (expired) {
-                    throw new Error("Timed out waiting for a response from the Discord owners");
+                    reject(Error("Timed out waiting for a response from the Discord owners"));
                 } else {
-                    throw new Error("The bridge has been declined by the Discord guild");
+                    reject(Error("The bridge has been declined by the Discord guild"));
                 }
             }
         };
 
-        this.pendingRequests[channelId] = approveFn;
-        setTimeout(() => approveFn(false, true), PERMISSION_REQUEST_TIMEOUT);
+        this.pendingRequests.set(channelId, approveFn);
+        setTimeout(() => approveFn(false, true), timeout);
 
-        await channel.sendMessage(`${requestor} on matrix would like to bridge this channel. Someone with permission` +
+        await channel.send(`${requestor} on matrix would like to bridge this channel. Someone with permission` +
             " to manage webhooks please reply with !approve or !deny in the next 5 minutes");
+        return await deferP;
+
     }
 
     public HasPendingRequest(channel: Discord.TextChannel): boolean {
         const channelId = `${channel.guild.id}/${channel.id}`;
-        return !!this.pendingRequests[channelId];
+        return this.pendingRequests.has(channelId);
     }
 
     public async MarkApproved(
@@ -72,17 +81,17 @@ export class Provisioner {
         allow: boolean,
     ): Promise<boolean> {
         const channelId = `${channel.guild.id}/${channel.id}`;
-        if (!this.pendingRequests[channelId]) {
+        if (!this.pendingRequests.has(channelId)) {
             return false; // no change, so false
         }
 
         const perms = channel.permissionsFor(member);
-        if (!perms || !perms.hasPermission(Discord.Permissions.FLAGS.MANAGE_WEBHOOKS as Discord.PermissionResolvable)) {
+        if (!perms || !perms.has(Discord.Permissions.FLAGS.MANAGE_WEBHOOKS as Discord.PermissionResolvable)) {
             // Missing permissions, so just reject it
             throw new Error("You do not have permission to manage webhooks in this channel");
         }
 
-        this.pendingRequests[channelId](allow);
+        this.pendingRequests.get(channelId)!(allow);
         return true; // replied, so true
     }
 }

test/mocks/channel.ts

 import {MockMember} from "./member";
 import {MockCollection} from "./collection";
+import {Permissions, PermissionResolvable} from "discord.js";
 
 // we are a test file and thus need those
 /* tslint:disable:no-unused-expression max-file-line-count no-any */
@@ -14,7 +15,12 @@ export class MockChannel {
         public name: string = "",
         public topic: string = "",
     ) { }
+
     public async send(data: any): Promise<any> {
         return data;
     }
+
+    public permissionsFor(member: MockMember) {
+        return new Permissions(Permissions.FLAGS.MANAGE_WEBHOOKS as PermissionResolvable);
+    }
 }

test/test_provisioner.ts

+import * as Chai from "chai";
+import { Provisioner } from "../src/provisioner";
+import { MockChannel } from "./mocks/channel";
+import { MockMember } from "./mocks/member";
+
+// we are a test file and thus need those
+/* tslint:disable:no-any */
+
+const expect = Chai.expect;
+
+const TIMEOUT_MS = 1000;
+
+describe("Provisioner", () => {
+    describe("AskBridgePermission", () => {
+        it("should fail to bridge a room that timed out", async () => {
+            const p = new Provisioner();
+            const startAt = Date.now();
+            try {
+                await p.AskBridgePermission(
+                    new MockChannel("foo", "bar") as any,
+                    "Mark",
+                    TIMEOUT_MS,
+                );
+                throw Error("Should have thrown an error");
+            } catch (err) {
+                expect(err.message).to.eq("Timed out waiting for a response from the Discord owners");
+                const delay = Date.now() - startAt;
+                if (delay < TIMEOUT_MS) {
+                    throw Error(`Should have waited for timeout before resolving, waited: ${delay}ms`);
+                }
+            }
+        });
+        it("should fail to bridge a room that was declined", async () => {
+            const p = new Provisioner();
+            const promise = p.AskBridgePermission(
+                new MockChannel("foo", "bar") as any,
+                "Mark",
+                TIMEOUT_MS,
+            );
+            await p.MarkApproved(new MockChannel("foo", "bar") as any, new MockMember("abc", "Mark") as any, false);
+            try {
+                await promise;
+                throw Error("Should have thrown an error");
+            } catch (err) {
+                expect(err.message).to.eq("The bridge has been declined by the Discord guild");
+            }
+
+        });
+        it("should bridge a room that was approved", async () => {
+            const p = new Provisioner();
+            const promise = p.AskBridgePermission(
+                new MockChannel("foo", "bar") as any,
+                "Mark",
+                TIMEOUT_MS,
+            );
+            await p.MarkApproved(new MockChannel("foo", "bar") as any, new MockMember("abc", "Mark") as any, true);
+            expect(await promise).to.eq("Approved");
+        });
+    });
+});