Track extension provisions and check bundled for routing (#3187)

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

Makefile

@@ -70,7 +70,7 @@ integration-win: binaries/client build-extension-types build-extensions
 .PHONY: build
 build: node_modules binaries/client
 	yarn run npm:fix-build-version
-	$(MAKE) build-extensions
+	$(MAKE) build-extensions -B
 	yarn run compile
 ifeq "$(DETECTED_OS)" "Windows"
 	yarn run electron-builder --publish onTag --x64 --ia32

src/common/protocol-handler/router.ts

@@ -31,6 +31,7 @@ import { ExtensionLoader } from "../../extensions/extension-loader";
 import type { LensExtension } from "../../extensions/lens-extension";
 import type { RouteHandler, RouteParams } from "../../extensions/registries/protocol-handler";
 import { when } from "mobx";
+import { ipcRenderer } from "electron";
 
 // IPC channel for protocol actions. Main broadcasts the open-url events to this channel.
 export const ProtocolHandlerIpcPrefix = "protocol-handler";
@@ -182,7 +183,10 @@ export abstract class LensProtocolRouter extends Singleton {
     const extensionLoader = ExtensionLoader.getInstance();
 
     try {
-      await when(() => !!extensionLoader.getInstanceByName(name), { timeout: 5_000 });
+      /**
+       * Note, if `getInstanceByName` returns `null` that means we won't be getting an instance
+       */
+      await when(() => extensionLoader.getInstanceByName(name) !== (void 0), { timeout: 5_000 });
     } catch(error) {
       logger.info(`${LensProtocolRouter.LoggingPrefix}: Extension ${name} matched, but not installed (${error})`);
 
@@ -191,7 +195,13 @@ export abstract class LensProtocolRouter extends Singleton {
 
     const extension = extensionLoader.getInstanceByName(name);
 
-    if (!ExtensionsStore.getInstance().isEnabled(extension.id)) {
+    if (!extension) {
+      logger.info(`${LensProtocolRouter.LoggingPrefix}: Extension ${name} matched, but does not have a class for ${ipcRenderer ? "renderer" : "main"}`);
+
+      return name;
+    }
+
+    if (!ExtensionsStore.getInstance().isEnabled(extension)) {
       logger.info(`${LensProtocolRouter.LoggingPrefix}: Extension ${name} matched, but not enabled`);
 
       return name;

src/extensions/extension-discovery.ts

@@ -357,8 +357,8 @@ export class ExtensionDiscovery extends Singleton {
   protected async getByManifest(manifestPath: string, { isBundled = false } = {}): Promise<InstalledExtension | null> {
     try {
       const manifest = await fse.readJson(manifestPath) as LensExtensionManifest;
-      const installedManifestPath = this.getInstalledManifestPath(manifest.name);
-      const isEnabled = isBundled || ExtensionsStore.getInstance().isEnabled(installedManifestPath);
+      const id = this.getInstalledManifestPath(manifest.name);
+      const isEnabled = ExtensionsStore.getInstance().isEnabled({ id, isBundled });
       const extensionDir = path.dirname(manifestPath);
       const npmPackage = path.join(extensionDir, `${manifest.name}-${manifest.version}.tgz`);
       const absolutePath = (isProduction && await fse.pathExists(npmPackage)) ? npmPackage : extensionDir;
@@ -369,9 +369,9 @@ export class ExtensionDiscovery extends Singleton {
       }
 
       return {
-        id: installedManifestPath,
+        id,
         absolutePath,
-        manifestPath: installedManifestPath,
+        manifestPath: id,
         manifest,
         isBundled,
         isEnabled,

src/extensions/extension-loader.ts

@@ -48,6 +48,13 @@ export class ExtensionLoader extends Singleton {
   protected extensions = observable.map<LensExtensionId, InstalledExtension>();
   protected instances = observable.map<LensExtensionId, LensExtension>();
 
+  /**
+   * This is the set of extensions that don't come with either
+   * - Main.LensExtension when running in the main process
+   * - Renderer.LensExtension when running in the renderer process
+   */
+  protected nonInstancesByName = observable.set<string>();
+
   /**
    * This is updated by the `observe` in the constructor. DO NOT write directly to it
    */
@@ -101,7 +108,20 @@ export class ExtensionLoader extends Singleton {
     return extensions;
   }
 
-  getInstanceByName(name: string): LensExtension | undefined {
+  /**
+   * Get the extension instance by its manifest name
+   * @param name The name of the extension
+   * @returns one of the following:
+   * - the instance of `Main.LensExtension` on the main process if created
+   * - the instance of `Renderer.LensExtension` on the renderer process if created
+   * - `null` if no class definition is provided for the current process
+   * - `undefined` if the name is not known about
+   */
+  getInstanceByName(name: string): LensExtension | null | undefined {
+    if (this.nonInstancesByName.has(name)) {
+      return null;
+    }
+
     return this.instancesByName.get(name);
   }
 
@@ -145,6 +165,7 @@ export class ExtensionLoader extends Singleton {
     this.extensions.set(extension.id, extension);
   }
 
+  @action
   removeInstance(lensExtensionId: LensExtensionId) {
     logger.info(`${logModule} deleting extension instance ${lensExtensionId}`);
     const instance = this.instances.get(lensExtensionId);
@@ -157,6 +178,7 @@ export class ExtensionLoader extends Singleton {
       instance.disable();
       this.events.emit("remove", instance);
       this.instances.delete(lensExtensionId);
+      this.nonInstancesByName.delete(instance.name);
     } catch (error) {
       logger.error(`${logModule}: deactivation extension error`, { lensExtensionId, error });
     }
@@ -302,13 +324,14 @@ export class ExtensionLoader extends Singleton {
   protected autoInitExtensions(register: (ext: LensExtension) => Promise<Disposer[]>) {
     return reaction(() => this.toJSON(), installedExtensions => {
       for (const [extId, extension] of installedExtensions) {
-        const alreadyInit = this.instances.has(extId);
+        const alreadyInit = this.instances.has(extId) || this.nonInstancesByName.has(extension.manifest.name);
 
         if (extension.isCompatible && extension.isEnabled && !alreadyInit) {
           try {
             const LensExtensionClass = this.requireExtension(extension);
 
             if (!LensExtensionClass) {
+              this.nonInstancesByName.add(extension.manifest.name);
               continue;
             }
 

src/extensions/extensions-store.ts

@@ -51,12 +51,10 @@ export class ExtensionsStore extends BaseStore<LensExtensionsStoreModel> {
 
   protected state = observable.map<LensExtensionId, LensExtensionState>();
 
-  isEnabled(extId: LensExtensionId): boolean {
-    const state = this.state.get(extId);
-
+  isEnabled({ id, isBundled }: { id: string, isBundled: boolean }): boolean {
     // By default false, so that copied extensions are disabled by default.
     // If user installs the extension from the UI, the Extensions component will specifically enable it.
-    return Boolean(state?.enabled);
+    return isBundled || Boolean(this.state.get(id)?.enabled);
   }
 
   @action

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

@@ -67,21 +67,21 @@ describe("protocol router tests", () => {
     mockFs.restore();
   });
 
-  it("should throw on non-lens URLS", () => {
+  it("should throw on non-lens URLS", async () => {
     try {
       const lpr = LensProtocolRouterMain.getInstance();
 
-      expect(lpr.route("https://google.ca")).toBeUndefined();
+      expect(await lpr.route("https://google.ca")).toBeUndefined();
     } catch (error) {
       expect(error).toBeInstanceOf(Error);
     }
   });
 
-  it("should throw when host not internal or extension", () => {
+  it("should throw when host not internal or extension", async () => {
     try {
       const lpr = LensProtocolRouterMain.getInstance();
 
-      expect(lpr.route("lens://foobar")).toBeUndefined();
+      expect(await lpr.route("lens://foobar")).toBeUndefined();
     } catch (error) {
       expect(error).toBeInstanceOf(Error);
     }
@@ -114,13 +114,13 @@ describe("protocol router tests", () => {
     lpr.addInternalHandler("/", noop);
 
     try {
-      expect(lpr.route("lens://app")).toBeUndefined();
+      expect(await lpr.route("lens://app")).toBeUndefined();
     } catch (error) {
       expect(throwIfDefined(error)).not.toThrow();
     }
 
     try {
-      expect(lpr.route("lens://extension/@mirantis/minikube")).toBeUndefined();
+      expect(await lpr.route("lens://extension/@mirantis/minikube")).toBeUndefined();
     } catch (error) {
       expect(throwIfDefined(error)).not.toThrow();
     }
@@ -130,14 +130,14 @@ describe("protocol router tests", () => {
     expect(broadcastMessage).toHaveBeenCalledWith(ProtocolHandlerExtension, "lens://extension/@mirantis/minikube", "matched");
   });
 
-  it("should call handler if matches", () => {
+  it("should call handler if matches", async () => {
     const lpr = LensProtocolRouterMain.getInstance();
     let called = false;
 
     lpr.addInternalHandler("/page", () => { called = true; });
 
     try {
-      expect(lpr.route("lens://app/page")).toBeUndefined();
+      expect(await lpr.route("lens://app/page")).toBeUndefined();
     } catch (error) {
       expect(throwIfDefined(error)).not.toThrow();
     }
@@ -146,7 +146,7 @@ describe("protocol router tests", () => {
     expect(broadcastMessage).toBeCalledWith(ProtocolHandlerInternal, "lens://app/page", "matched");
   });
 
-  it("should call most exact handler", () => {
+  it("should call most exact handler", async () => {
     const lpr = LensProtocolRouterMain.getInstance();
     let called: any = 0;
 
@@ -154,7 +154,7 @@ describe("protocol router tests", () => {
     lpr.addInternalHandler("/page/:id", params => { called = params.pathname.id; });
 
     try {
-      expect(lpr.route("lens://app/page/foo")).toBeUndefined();
+      expect(await lpr.route("lens://app/page/foo")).toBeUndefined();
     } catch (error) {
       expect(throwIfDefined(error)).not.toThrow();
     }
@@ -194,7 +194,7 @@ describe("protocol router tests", () => {
     (ExtensionsStore.getInstance() as any).state.set(extId, { enabled: true, name: "@foobar/icecream" });
 
     try {
-      expect(lpr.route("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();
     }
@@ -262,7 +262,7 @@ describe("protocol router tests", () => {
     (ExtensionsStore.getInstance() as any).state.set("icecream", { enabled: true, name: "icecream" });
 
     try {
-      expect(lpr.route("lens://extension/icecream/page")).toBeUndefined();
+      expect(await lpr.route("lens://extension/icecream/page")).toBeUndefined();
     } catch (error) {
       expect(throwIfDefined(error)).not.toThrow();
     }
@@ -279,7 +279,7 @@ describe("protocol router tests", () => {
     expect(() => lpr.addInternalHandler("/:@", noop)).toThrowError();
   });
 
-  it("should call most exact handler with 3 found handlers", () => {
+  it("should call most exact handler with 3 found handlers", async () => {
     const lpr = LensProtocolRouterMain.getInstance();
     let called: any = 0;
 
@@ -289,7 +289,7 @@ describe("protocol router tests", () => {
     lpr.addInternalHandler("/page/bar", () => { called = 4; });
 
     try {
-      expect(lpr.route("lens://app/page/foo/bar/bat")).toBeUndefined();
+      expect(await lpr.route("lens://app/page/foo/bar/bat")).toBeUndefined();
     } catch (error) {
       expect(throwIfDefined(error)).not.toThrow();
     }
@@ -298,7 +298,7 @@ describe("protocol router tests", () => {
     expect(broadcastMessage).toBeCalledWith(ProtocolHandlerInternal, "lens://app/page/foo/bar/bat", "matched");
   });
 
-  it("should call most exact handler with 2 found handlers", () => {
+  it("should call most exact handler with 2 found handlers", async () => {
     const lpr = LensProtocolRouterMain.getInstance();
     let called: any = 0;
 
@@ -307,7 +307,7 @@ describe("protocol router tests", () => {
     lpr.addInternalHandler("/page/bar", () => { called = 4; });
 
     try {
-      expect(lpr.route("lens://app/page/foo/bar/bat")).toBeUndefined();
+      expect(await lpr.route("lens://app/page/foo/bar/bat")).toBeUndefined();
     } catch (error) {
       expect(throwIfDefined(error)).not.toThrow();
     }

src/main/protocol-handler/router.ts

@@ -73,7 +73,7 @@ export class LensProtocolRouterMain extends proto.LensProtocolRouter {
    * This will send an IPC message to the renderer router to do the same
    * in the renderer.
    */
-  public route(rawUrl: string) {
+  public async route(rawUrl: string) {
     try {
       const url = new URLParse(rawUrl, true);
 
@@ -89,7 +89,7 @@ export class LensProtocolRouterMain extends proto.LensProtocolRouter {
       if (routeInternally) {
         this._routeToInternal(url);
       } else {
-        this._routeToExtension(url);
+        await this._routeToExtension(url);
       }
     } catch (error) {
       broadcastMessage(ProtocolHandlerInvalid, error.toString(), rawUrl);