Merge pull request #4527 from jansav/make-menu-registry-obsolete

Remove MenuRegistry in favour of dependency injection

package.json

@@ -196,8 +196,8 @@
     "@hapi/call": "^8.0.1",
     "@hapi/subtext": "^7.0.3",
     "@kubernetes/client-node": "^0.16.1",
-    "@ogre-tools/injectable": "^1.3.0",
-    "@ogre-tools/injectable-react": "^1.3.1",
+    "@ogre-tools/injectable": "^1.4.1",
+    "@ogre-tools/injectable-react": "^1.4.1",
     "@sentry/electron": "^2.5.4",
     "@sentry/integrations": "^6.15.0",
     "abort-controller": "^3.0.0",

src/common/protocol-handler/router.ts

@@ -21,13 +21,13 @@
 
 import { match, matchPath } from "react-router";
 import { countBy } from "lodash";
-import { iter, Singleton } from "../utils";
+import { iter } from "../utils";
 import { pathToRegexp } from "path-to-regexp";
 import logger from "../../main/logger";
 import type Url from "url-parse";
 import { RoutingError, RoutingErrorType } from "./error";
 import { ExtensionsStore } from "../../extensions/extensions-store";
-import { ExtensionLoader } from "../../extensions/extension-loader";
+import type { ExtensionLoader as ExtensionLoaderType } from "../../extensions/extension-loader/extension-loader";
 import type { LensExtension } from "../../extensions/lens-extension";
 import type { RouteHandler, RouteParams } from "../../extensions/registries/protocol-handler";
 import { when } from "mobx";
@@ -78,14 +78,20 @@ export function foldAttemptResults(mainAttempt: RouteAttempt, rendererAttempt: R
   }
 }
 
-export abstract class LensProtocolRouter extends Singleton {
+interface Dependencies {
+  extensionLoader: ExtensionLoaderType
+}
+
+export abstract class LensProtocolRouter {
   // Map between path schemas and the handlers
   protected internalRoutes = new Map<string, RouteHandler>();
 
   public static readonly LoggingPrefix = "[PROTOCOL ROUTER]";
 
   static readonly ExtensionUrlSchema = `/:${EXTENSION_PUBLISHER_MATCH}(\@[A-Za-z0-9_]+)?/:${EXTENSION_NAME_MATCH}`;
 
+  constructor(protected dependencies: Dependencies) {}
+
   /**
    * Attempts to route the given URL to all internal routes that have been registered
    * @param url the parsed URL that initiated the `lens://` protocol
@@ -180,15 +186,20 @@ export abstract class LensProtocolRouter extends Singleton {
 
     const { [EXTENSION_PUBLISHER_MATCH]: publisher, [EXTENSION_NAME_MATCH]: partialName } = match.params;
     const name = [publisher, partialName].filter(Boolean).join("/");
-    const extensionLoader = ExtensionLoader.getInstance();
+
+    const extensionLoader = this.dependencies.extensionLoader;
 
     try {
       /**
        * 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})`);
+      await when(() => extensionLoader.getInstanceByName(name) !== void 0, {
+        timeout: 5_000,
+      });
+    } catch (error) {
+      logger.info(
+        `${LensProtocolRouter.LoggingPrefix}: Extension ${name} matched, but not installed (${error})`,
+      );
 
       return name;
     }
@@ -233,7 +244,6 @@ 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 + 1));
 
-
     try {
       const handlers = iter.map(extension.protocolHandlers, ({ pathSchema, handler }) => [pathSchema, handler] as [string, RouteHandler]);
 

src/extensions/__tests__/extension-discovery.test.ts

@@ -27,6 +27,9 @@ import { ExtensionDiscovery } from "../extension-discovery";
 import os from "os";
 import { Console } from "console";
 import { AppPaths } from "../../common/app-paths";
+import type { ExtensionLoader } from "../extension-loader";
+import extensionLoaderInjectable from "../extension-loader/extension-loader.injectable";
+import { getDiForUnitTesting } from "../getDiForUnitTesting";
 
 jest.setTimeout(60_000);
 
@@ -62,10 +65,16 @@ console = new Console(process.stdout, process.stderr); // fix mockFS
 const mockedWatch = watch as jest.MockedFunction<typeof watch>;
 
 describe("ExtensionDiscovery", () => {
+  let extensionLoader: ExtensionLoader;
+
   beforeEach(() => {
     ExtensionDiscovery.resetInstance();
     ExtensionsStore.resetInstance();
     ExtensionsStore.createInstance();
+
+    const di = getDiForUnitTesting();
+
+    extensionLoader = di.inject(extensionLoaderInjectable);
   });
 
   describe("with mockFs", () => {
@@ -98,7 +107,9 @@ describe("ExtensionDiscovery", () => {
         (mockWatchInstance) as any,
       );
 
-      const extensionDiscovery = ExtensionDiscovery.createInstance();
+      const extensionDiscovery = ExtensionDiscovery.createInstance(
+        extensionLoader,
+      );
 
       // Need to force isLoaded to be true so that the file watching is started
       extensionDiscovery.isLoaded = true;
@@ -140,7 +151,9 @@ describe("ExtensionDiscovery", () => {
     mockedWatch.mockImplementationOnce(() =>
       (mockWatchInstance) as any,
     );
-    const extensionDiscovery = ExtensionDiscovery.createInstance();
+    const extensionDiscovery = ExtensionDiscovery.createInstance(
+      extensionLoader,
+    );
 
     // Need to force isLoaded to be true so that the file watching is started
     extensionDiscovery.isLoaded = true;

src/extensions/__tests__/extension-loader.test.ts

@@ -19,11 +19,13 @@
  * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
  */
 
-import { ExtensionLoader } from "../extension-loader";
+import type { ExtensionLoader } from "../extension-loader";
 import { ipcRenderer } from "electron";
 import { ExtensionsStore } from "../extensions-store";
 import { Console } from "console";
 import { stdout, stderr } from "process";
+import { getDiForUnitTesting } from "../getDiForUnitTesting";
+import extensionLoaderInjectable from "../extension-loader/extension-loader.injectable";
 
 console = new Console(stdout, stderr);
 
@@ -128,13 +130,15 @@ jest.mock(
 );
 
 describe("ExtensionLoader", () => {
+  let extensionLoader: ExtensionLoader;
+
   beforeEach(() => {
-    ExtensionLoader.resetInstance();
-  });
+    const di = getDiForUnitTesting();
 
-  it.only("renderer updates extension after ipc broadcast", async (done) => {
-    const extensionLoader = ExtensionLoader.createInstance();
+    extensionLoader = di.inject(extensionLoaderInjectable);
+  });
 
+  it.only("renderer updates extension after ipc broadcast", async done => {
     expect(extensionLoader.userExtensions).toMatchInlineSnapshot(`Map {}`);
 
     await extensionLoader.init();
@@ -178,8 +182,6 @@ describe("ExtensionLoader", () => {
     // Disable sending events in this test
     (ipcRenderer.on as any).mockImplementation();
 
-    const extensionLoader = ExtensionLoader.createInstance();
-
     await extensionLoader.init();
 
     expect(ExtensionsStore.getInstance().mergeState).not.toHaveBeenCalled();
@@ -194,6 +196,7 @@ describe("ExtensionLoader", () => {
       "manifest/path2": {
         enabled: true,
         name: "TestExtension2",
-      }});
+      },
+    });
   });
 });

src/extensions/extension-discovery.ts

@@ -32,7 +32,7 @@ import logger from "../main/logger";
 import { ExtensionInstallationStateStore } from "../renderer/components/+extensions/extension-install.store";
 import { extensionInstaller } from "./extension-installer";
 import { ExtensionsStore } from "./extensions-store";
-import { ExtensionLoader } from "./extension-loader";
+import type { ExtensionLoader } from "./extension-loader";
 import type { LensExtensionId, LensExtensionManifest } from "./lens-extension";
 import { isProduction } from "../common/vars";
 import { isCompatibleBundledExtension, isCompatibleExtension } from "./extension-compatibility";
@@ -99,7 +99,7 @@ export class ExtensionDiscovery extends Singleton {
 
   public events = new EventEmitter();
 
-  constructor() {
+  constructor(protected extensionLoader: ExtensionLoader) {
     super();
 
     makeObservable(this);
@@ -277,7 +277,7 @@ export class ExtensionDiscovery extends Singleton {
    * @param extensionId The ID of the extension to uninstall.
    */
   async uninstallExtension(extensionId: LensExtensionId): Promise<void> {
-    const { manifest, absolutePath } = this.extensions.get(extensionId) ?? ExtensionLoader.getInstance().getExtension(extensionId);
+    const { manifest, absolutePath } = this.extensions.get(extensionId) ?? this.extensionLoader.getExtension(extensionId);
 
     logger.info(`${logModule} Uninstalling ${manifest.name}`);
 

src/extensions/extension-loader/extension-loader.injectable.ts

@@ -0,0 +1,31 @@
+/**
+ * Copyright (c) 2021 OpenLens Authors
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy of
+ * this software and associated documentation files (the "Software"), to deal in
+ * the Software without restriction, including without limitation the rights to
+ * use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of
+ * the Software, and to permit persons to whom the Software is furnished to do so,
+ * subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in all
+ * copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
+ * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
+ * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER
+ * IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+ */
+import type { Injectable } from "@ogre-tools/injectable";
+import { lifecycleEnum } from "@ogre-tools/injectable";
+import { ExtensionLoader } from "./extension-loader";
+
+const extensionLoaderInjectable: Injectable<ExtensionLoader> = {
+  getDependencies: () => ({}),
+  instantiate: () => new ExtensionLoader(),
+  lifecycle: lifecycleEnum.singleton,
+};
+
+export default extensionLoaderInjectable;

src/extensions/extension-loader.ts → src/extensions/extension-loader/extension-loader.ts

@@ -24,17 +24,16 @@ import { EventEmitter } from "events";
 import { isEqual } from "lodash";
 import { action, computed, makeObservable, observable, observe, reaction, when } from "mobx";
 import path from "path";
-import { AppPaths } from "../common/app-paths";
-import { ClusterStore } from "../common/cluster-store";
-import { broadcastMessage, ipcMainOn, ipcRendererOn, requestMain, ipcMainHandle } from "../common/ipc";
-import { Disposer, getHostedClusterId, Singleton, toJS } from "../common/utils";
-import logger from "../main/logger";
-import type { InstalledExtension } from "./extension-discovery";
-import { ExtensionsStore } from "./extensions-store";
-import type { LensExtension, LensExtensionConstructor, LensExtensionId } from "./lens-extension";
-import type { LensMainExtension } from "./lens-main-extension";
-import type { LensRendererExtension } from "./lens-renderer-extension";
-import * as registries from "./registries";
+import { AppPaths } from "../../common/app-paths";
+import { ClusterStore } from "../../common/cluster-store";
+import { broadcastMessage, ipcMainOn, ipcRendererOn, requestMain, ipcMainHandle } from "../../common/ipc";
+import { Disposer, getHostedClusterId, toJS } from "../../common/utils";
+import logger from "../../main/logger";
+import type { InstalledExtension } from "../extension-discovery";
+import { ExtensionsStore } from "../extensions-store";
+import type { LensExtension, LensExtensionConstructor, LensExtensionId } from "../lens-extension";
+import type { LensRendererExtension } from "../lens-renderer-extension";
+import * as registries from "../registries";
 
 export function extensionPackagesRoot() {
   return path.join(AppPaths.get("userData"));
@@ -45,7 +44,7 @@ const logModule = "[EXTENSIONS-LOADER]";
 /**
  * Loads installed extensions to the Lens application
  */
-export class ExtensionLoader extends Singleton {
+export class ExtensionLoader {
   protected extensions = observable.map<LensExtensionId, InstalledExtension>();
   protected instances = observable.map<LensExtensionId, LensExtension>();
 
@@ -77,7 +76,6 @@ export class ExtensionLoader extends Singleton {
   }
 
   constructor() {
-    super();
     makeObservable(this);
     observe(this.instances, change => {
       switch (change.type) {
@@ -97,6 +95,10 @@ export class ExtensionLoader extends Singleton {
     });
   }
 
+  @computed get enabledExtensionInstances() : LensExtension[] {
+    return [...this.instances.values()].filter(extension => extension.isEnabled);
+  }
+
   @computed get userExtensions(): Map<LensExtensionId, InstalledExtension> {
     const extensions = this.toJSON();
 
@@ -248,25 +250,7 @@ export class ExtensionLoader extends Singleton {
   }
 
   loadOnMain() {
-    registries.MenuRegistry.createInstance();
-
-    logger.debug(`${logModule}: load on main`);
-    this.autoInitExtensions(async (extension: LensMainExtension) => {
-      // Each .add returns a function to remove the item
-      const removeItems = [
-        registries.MenuRegistry.getInstance().add(extension.appMenus),
-      ];
-
-      this.events.on("remove", (removedExtension: LensRendererExtension) => {
-        if (removedExtension.id === extension.id) {
-          removeItems.forEach(remove => {
-            remove();
-          });
-        }
-      });
-
-      return removeItems;
-    });
+    this.autoInitExtensions(() => Promise.resolve([]));
   }
 
   loadOnClusterManagerRenderer() {

src/renderer/components/kube-object-menu/kube-object-menu-container.tsx → src/extensions/extension-loader/index.ts

@@ -18,7 +18,5 @@
  * IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
  * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
  */
-import { getInjectedComponent } from "@ogre-tools/injectable-react";
-import KubeObjectMenuInjectable from "./kube-object-menu.injectable";
 
-export const KubeObjectMenu = getInjectedComponent(KubeObjectMenuInjectable);
+export * from "./extension-loader";

src/extensions/extensions.injectable.ts

@@ -0,0 +1,41 @@
+/**
+ * Copyright (c) 2021 OpenLens Authors
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy of
+ * this software and associated documentation files (the "Software"), to deal in
+ * the Software without restriction, including without limitation the rights to
+ * use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of
+ * the Software, and to permit persons to whom the Software is furnished to do so,
+ * subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in all
+ * copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
+ * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
+ * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER
+ * IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+ */
+import { Injectable, lifecycleEnum } from "@ogre-tools/injectable";
+import { computed, IComputedValue } from "mobx";
+import type { LensExtension } from "./lens-extension";
+import type { ExtensionLoader } from "./extension-loader";
+import extensionLoaderInjectable from "./extension-loader/extension-loader.injectable";
+
+const extensionsInjectable: Injectable<
+  IComputedValue<LensExtension[]>,
+  { extensionLoader: ExtensionLoader }
+> = {
+  getDependencies: di => ({
+    extensionLoader: di.inject(extensionLoaderInjectable),
+  }),
+
+  lifecycle: lifecycleEnum.singleton,
+
+  instantiate: ({ extensionLoader }) =>
+    computed(() => extensionLoader.enabledExtensionInstances),
+};
+
+export default extensionsInjectable;