Fix unit tests and update docs

Signed-off-by: Sebastian Malton <sebastian@malton.name>

docs/extensions/guides/protocol-handlers.md

@@ -6,11 +6,21 @@ Lens provides a routing mechanism that extensions can use to register custom han
 
 ## Registering A Protocol Handler
 
-The method `onProtocolRequest` exists both on [`LensMainExtension`](extensions/api/classes/lensmainextension/#onprotocolrequest) and on [`LensRendererExtension`](extensions/api/classes/lensrendererextension/#onprotocolrequest).
+The field `protocolhandlers` exists both on [`LensMainExtension`](extensions/api/classes/lensmainextension/#protocolhandlers) and on [`LensRendererExtension`](extensions/api/classes/lensrendererextension/#protocolhandlers).
-This is how, as an extension developer, you can register handlers for your extension.
+This field will be iterated through every time a `lens://` request gets sent to the application.
 The `pathSchema` argument must comply with the [path-to-regexp](https://www.npmjs.com/package/path-to-regexp) package's `compileToRegex` function.
 
 Once you have registered a handler it will be called when a user opens a link on their computer.
+Handlers will be run in both `main` and `renderer` in parallel with no synchronization between the two processes.
+Furthermore, both `main` and `renderer` are routed separately.
+In other words, which handler is selected in either process is independent from the list of possible handlers in the other.
+
+## Deregistering A Protocol Handler
+
+All that is needed to deregister a handler is to remove it from the array of handlers.
+
+## Routing Algorithm
+
 The routing mechanism for extensions is quite straight forward.
 For example consider an extension `example-extension` which is published by the `@mirantis` org.
 If it were to register a handler with `"/display/:type"` as its corresponding link then we would match the following URI like this:
@@ -45,9 +55,3 @@ On the other hand, the subpath `"/display/notification"` would be routed to #3.
 
 The URI is routed to the most specific matching `pathSchema`.
 This way the `"/"` (root) `pathSchema` acts as a sort of catch all or default route if no other route matches.
-
-### Cleaning Up
-
-Currently there is not way to remove a protocol handler once it has been registered.
-Handlers will not be called if the extension is deactivated or uninstalled.
-This means that the handlers should be added (or re-added as the case may be) on every activation of an extension instance.

src/common/protocol-handler/router.ts

@@ -191,7 +191,7 @@ export abstract class LensProtocolRouter extends Singleton {
     }
 
     // remove the extension name from the path name so we don't need to match on it anymore
-    url.set("pathname", url.pathname.slice(extension.name.length));
+    url.set("pathname", url.pathname.slice(extension.name.length + 1));
 
     const handlers = extension
       .protocolHandlers

src/extensions/extension-loader.ts

@@ -55,7 +55,7 @@ export class ExtensionLoader {
   @computed get userExtensionsByName(): Map<string, LensExtension> {
     const res = new Map();
 
-    for (const [, val] of this.instances) {
+    for (const [, val] of this.instances.toJS()) {
       if (val.isBundled) {
         continue;
       }

src/extensions/lens-main-extension.ts

@@ -2,8 +2,6 @@ import type { MenuRegistration } from "./registries/menu-registry";
 import { LensExtension } from "./lens-extension";
 import { WindowManager } from "../main/window-manager";
 import { getExtensionPageUrl } from "./registries/page-registry";
-import { RouteHandler } from "../common/protocol-handler";
-import { LensProtocolRouterMain } from "../main/protocol-handler";
 
 export class LensMainExtension extends LensExtension {
   appMenus: MenuRegistration[] = [];
@@ -18,23 +16,4 @@ export class LensMainExtension extends LensExtension {
 
     await windowManager.navigate(pageUrl, frameId);
   }
-
-  async disable() {
-    const lprm = LensProtocolRouterMain.getInstance<LensProtocolRouterMain>();
-
-    lprm.removeExtensionHandlers(this.name);
-
-    return super.disable();
-  }
-
-  /**
-   * Registers a handler to be called when a `lens://` link is called.
-   * @param pathSchema The path schema for the route.
-   * @param handler The function to call when this route has been matched
-   */
-  onProtocolRequest(pathSchema: string, handler: RouteHandler): void {
-    const lprm = LensProtocolRouterMain.getInstance<LensProtocolRouterMain>();
-
-    lprm.extensionOn(this.name, pathSchema, handler);
-  }
 }

src/main/protocol-handler/__test__/router.test.ts

@@ -1,7 +1,14 @@
 import { LensProtocolRouterMain } from "../router";
-import Url from "url-parse";
 import { noop } from "../../../common/utils";
 import { extensionsStore } from "../../../extensions/extensions-store";
+import { extensionLoader } from "../../../extensions/extension-loader";
+import * as uuid from "uuid";
+import { LensMainExtension } from "../../../extensions/core-api";
+import { broadcastMessage } from "../../../common/ipc";
+import { ProtocolHandlerExtension, ProtocolHandlerInternal } from "../../../common/protocol-handler";
+import Url from "url-parse";
+
+jest.mock("../../../common/ipc");
 
 function throwIfDefined(val: any): void {
   if (val != null) {
@@ -13,14 +20,16 @@ describe("protocol router tests", () => {
   let lpr: LensProtocolRouterMain;
 
   beforeEach(() => {
+    jest.clearAllMocks();
     (extensionsStore as any).state.clear();
+    (extensionLoader as any).instances.clear();
     LensProtocolRouterMain.resetInstance();
     lpr = LensProtocolRouterMain.getInstance<LensProtocolRouterMain>();
   });
 
   it("should throw on non-lens URLS", async () => {
     try {
-      expect(await lpr.route(Url("https://google.ca"))).toBeUndefined();
+      expect(await lpr.route("https://google.ca")).toBeUndefined();
     } catch (error) {
       expect(error).toBeInstanceOf(Error);
     }
@@ -28,134 +37,222 @@ describe("protocol router tests", () => {
 
   it("should throw when host not internal or extension", async () => {
     try {
-      expect(await lpr.route(Url("lens://foobar"))).toBeUndefined();
+      expect(await lpr.route("lens://foobar")).toBeUndefined();
     } catch (error) {
       expect(error).toBeInstanceOf(Error);
     }
   });
 
   it("should not throw when has valid host", async () => {
-    (extensionsStore as any).state.set("@mirantis/minikube", { enabled: true, name: "@mirantis/minikube" });
+    const extId = uuid.v4();
-    lpr.on("/", noop);
+    const ext = new LensMainExtension({
-    lpr.extensionOn("@mirantis/minikube", "/", noop);
+      id: extId,
+      manifestPath: "/foo/bar",
+      manifest: {
+        name: "@mirantis/minikube",
+        version: "0.1.1",
+      },
+      isBundled: false,
+      isEnabled: true,
+      absolutePath: "/foo/bar",
+    });
+
+    ext.protocolHandlers.push({
+      pathSchema: "/",
+      handler: noop,
+    });
+
+    (extensionLoader as any).instances.set(extId, ext);
+    (extensionsStore as any).state.set(extId, { enabled: true, name: "@mirantis/minikube" });
+
+    lpr.addInternalHandler("/", noop);
 
     try {
-      expect(await lpr.route(Url("lens://internal"))).toBeUndefined();
+      expect(await lpr.route("lens://internal")).toBeUndefined();
     } catch (error) {
       expect(throwIfDefined(error)).not.toThrow();
-
     }
 
+
     try {
-      expect(await lpr.route(Url("lens://extension/@mirantis/minikube"))).toBeUndefined();
+      expect(await lpr.route("lens://extension/@mirantis/minikube")).toBeUndefined();
     } catch (error) {
       expect(throwIfDefined(error)).not.toThrow();
-
     }
+
+    expect(broadcastMessage).toHaveBeenNthCalledWith(1, ProtocolHandlerInternal, new Url("lens://internal", true));
+    expect(broadcastMessage).toHaveBeenNthCalledWith(2, ProtocolHandlerExtension, new Url("lens://extension/@mirantis/minikube", true));
   });
 
   it("should call handler if matches", async () => {
     let called = false;
 
-    lpr.on("/page", () => { called = true; });
+    lpr.addInternalHandler("/page", () => { called = true; });
 
     try {
-      expect(await lpr.route(Url("lens://internal/page"))).toBeUndefined();
+      expect(await lpr.route("lens://internal/page")).toBeUndefined();
     } catch (error) {
       expect(throwIfDefined(error)).not.toThrow();
-
     }
 
     expect(called).toBe(true);
+    expect(broadcastMessage).toBeCalledWith(ProtocolHandlerInternal, new Url("lens://internal/page", true));
   });
 
   it("should call most exact handler", async () => {
     let called: any = 0;
 
-    lpr.on("/page", () => { called = 1; });
+    lpr.addInternalHandler("/page", () => { called = 1; });
-    lpr.on("/page/:id", params => { called = params.pathname.id; });
+    lpr.addInternalHandler("/page/:id", params => { called = params.pathname.id; });
 
     try {
-      expect(await lpr.route(Url("lens://internal/page/foo"))).toBeUndefined();
+      expect(await lpr.route("lens://internal/page/foo")).toBeUndefined();
     } catch (error) {
       expect(throwIfDefined(error)).not.toThrow();
-
     }
 
     expect(called).toBe("foo");
+    expect(broadcastMessage).toBeCalledWith(ProtocolHandlerInternal, new Url("lens://internal/page/foo", true));
   });
 
   it("should call most exact handler for an extension", async () => {
-    (extensionsStore as any).state.set("@foobar/icecream", { enabled: true, name: "@foobar/icecream" });
     let called: any = 0;
 
-    lpr.extensionOn("@foobar/icecream", "/page", () => { called = 1; });
+    const extId = uuid.v4();
-    lpr.extensionOn("@foobar/icecream", "/page/:id", params => { called = params.pathname.id; });
+    const ext = new LensMainExtension({
+      id: extId,
+      manifestPath: "/foo/bar",
+      manifest: {
+        name: "@foobar/icecream",
+        version: "0.1.1",
+      },
+      isBundled: false,
+      isEnabled: true,
+      absolutePath: "/foo/bar",
+    });
+
+    ext.protocolHandlers
+      .push({
+        pathSchema: "/page",
+        handler: () => { called = 1; },
+      }, {
+        pathSchema: "/page/:id",
+        handler: params => { called = params.pathname.id; },
+      });
+
+    (extensionLoader as any).instances.set(extId, ext);
+    (extensionsStore as any).state.set(extId, { enabled: true, name: "@foobar/icecream" });
 
     try {
-      expect(await lpr.route(Url("lens://extension/@foobar/icecream/page/foob"))).toBeUndefined();
+      expect(await lpr.route("lens://extension/@foobar/icecream/page/foob")).toBeUndefined();
     } catch (error) {
       expect(throwIfDefined(error)).not.toThrow();
-
     }
 
     expect(called).toBe("foob");
+    expect(broadcastMessage).toBeCalledWith(ProtocolHandlerExtension, new Url("lens://extension/@foobar/icecream/page/foob", true));
   });
 
   it("should work with non-org extensions", async () => {
-    (extensionsStore as any).state.set("@foobar/icecream", { enabled: true, name: "@foobar/icecream" });
-    (extensionsStore as any).state.set("icecream", { enabled: true, name: "icecream" });
     let called: any = 0;
 
-    lpr.extensionOn("icecream", "/page", () => { called = 1; });
+    {
-    lpr.extensionOn("@foobar/icecream", "/page/:id", params => { called = params.pathname.id; });
+      const extId = uuid.v4();
+      const ext = new LensMainExtension({
+        id: extId,
+        manifestPath: "/foo/bar",
+        manifest: {
+          name: "@foobar/icecream",
+          version: "0.1.1",
+        },
+        isBundled: false,
+        isEnabled: true,
+        absolutePath: "/foo/bar",
+      });
+
+      ext.protocolHandlers
+        .push({
+          pathSchema: "/page/:id",
+          handler: params => { called = params.pathname.id; },
+        });
+
+      (extensionLoader as any).instances.set(extId, ext);
+      (extensionsStore as any).state.set(extId, { enabled: true, name: "@foobar/icecream" });
+    }
+
+    {
+      const extId = uuid.v4();
+      const ext = new LensMainExtension({
+        id: extId,
+        manifestPath: "/foo/bar",
+        manifest: {
+          name: "icecream",
+          version: "0.1.1",
+        },
+        isBundled: false,
+        isEnabled: true,
+        absolutePath: "/foo/bar",
+      });
+
+      ext.protocolHandlers
+        .push({
+          pathSchema: "/page",
+          handler: () => { called = 1; },
+        });
+
+      (extensionLoader as any).instances.set(extId, ext);
+      (extensionsStore as any).state.set(extId, { enabled: true, name: "icecream" });
+    }
+
+    (extensionsStore as any).state.set("@foobar/icecream", { enabled: true, name: "@foobar/icecream" });
+    (extensionsStore as any).state.set("icecream", { enabled: true, name: "icecream" });
 
     try {
-      expect(await lpr.route(Url("lens://extension/icecream/page"))).toBeUndefined();
+      expect(await lpr.route("lens://extension/icecream/page")).toBeUndefined();
     } catch (error) {
       expect(throwIfDefined(error)).not.toThrow();
     }
 
     expect(called).toBe(1);
+    expect(broadcastMessage).toBeCalledWith(ProtocolHandlerExtension, new Url("lens://extension/icecream/page", true));
   });
 
   it("should throw if urlSchema is invalid", () => {
-    expect(() => lpr.on("/:@", noop)).toThrowError();
+    expect(() => lpr.addInternalHandler("/:@", noop)).toThrowError();
-    expect(() => lpr.extensionOn("@foobar/icecream", "/page/:@", noop)).toThrowError();
   });
 
   it("should call most exact handler with 3 found handlers", async () => {
     let called: any = 0;
 
-    lpr.on("/", () => { called = 2; });
+    lpr.addInternalHandler("/", () => { called = 2; });
-    lpr.on("/page", () => { called = 1; });
+    lpr.addInternalHandler("/page", () => { called = 1; });
-    lpr.on("/page/foo", () => { called = 3; });
+    lpr.addInternalHandler("/page/foo", () => { called = 3; });
-    lpr.on("/page/bar", () => { called = 4; });
+    lpr.addInternalHandler("/page/bar", () => { called = 4; });
 
     try {
-      expect(await lpr.route(Url("lens://internal/page/foo/bar/bat"))).toBeUndefined();
+      expect(await lpr.route("lens://internal/page/foo/bar/bat")).toBeUndefined();
     } catch (error) {
       expect(throwIfDefined(error)).not.toThrow();
-
     }
 
     expect(called).toBe(3);
+    expect(broadcastMessage).toBeCalledWith(ProtocolHandlerInternal, new Url("lens://internal/page/foo/bar/bat", true));
   });
 
   it("should call most exact handler with 2 found handlers", async () => {
     let called: any = 0;
 
-    lpr.on("/", () => { called = 2; });
+    lpr.addInternalHandler("/", () => { called = 2; });
-    lpr.on("/page", () => { called = 1; });
+    lpr.addInternalHandler("/page", () => { called = 1; });
-    lpr.on("/page/bar", () => { called = 4; });
+    lpr.addInternalHandler("/page/bar", () => { called = 4; });
 
     try {
-      expect(await lpr.route(Url("lens://internal/page/foo/bar/bat"))).toBeUndefined();
+      expect(await lpr.route("lens://internal/page/foo/bar/bat")).toBeUndefined();
     } catch (error) {
       expect(throwIfDefined(error)).not.toThrow();
-
     }
 
     expect(called).toBe(1);
+    expect(broadcastMessage).toBeCalledWith(ProtocolHandlerInternal, new Url("lens://internal/page/foo/bar/bat", true));
   });
 });

src/main/protocol-handler/router.ts

@@ -1,8 +1,8 @@
 import logger from "../logger";
 import * as proto from "../../common/protocol-handler";
-import { WindowManager } from "../window-manager";
 import Url from "url-parse";
 import { LensExtension } from "../../extensions/lens-extension";
+import { broadcastMessage } from "../../common/ipc";
 
 export interface FallbackHandler {
   (name: string): Promise<boolean>;
@@ -70,23 +70,22 @@ export class LensProtocolRouterMain extends proto.LensProtocolRouter {
   }
 
   protected _routeToInternal(url: Url): void {
-    super._routeToExtension(url);
+    super._routeToInternal(url);
 
-    WindowManager.getInstance<WindowManager>().sendToView({
+    broadcastMessage(proto.ProtocolHandlerInternal, url);
-      channel: proto.ProtocolHandlerInternal,
-      data: [url],
-    });
   }
 
   protected async _routeToExtension(url: Url): Promise<void> {
-    // this needs to be done first, so that the missing extension handlers can
+    /**
-    // be called before notifying the renderer.
+     * This needs to be done first, so that the missing extension handlers can
-    await super._routeToExtension(url);
+     * be called before notifying the renderer.
+     *
+     * Note: this needs to clone the url because _routeToExtension modifies its
+     * argument.
+     */
+    await super._routeToExtension(new Url(url.toString(), true));
 
-    WindowManager.getInstance<WindowManager>().sendToView({
+    broadcastMessage(proto.ProtocolHandlerExtension, url);
-      channel: proto.ProtocolHandlerExtension,
-      data: [url],
-    });
   }
 
   /**