Switch to getting menu items by observing change in extensions instead local state

Signed-off-by: Janne Savolainen <janne.savolainen@live.fi>
Co-authored-by: Mikko Aspiala <mikko.aspiala@gmail.com>

src/extensions/extension-loader.ts

@@ -32,7 +32,6 @@ 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";
 
@@ -47,7 +46,7 @@ const logModule = "[EXTENSIONS-LOADER]";
  */
 export class ExtensionLoader extends Singleton {
   protected extensions = observable.map<LensExtensionId, InstalledExtension>();
-  protected instances = observable.map<LensExtensionId, LensExtension>();
+  instances = observable.map<LensExtensionId, LensExtension>();
 
   /**
    * This is the set of extensions that don't come with either
@@ -248,25 +247,7 @@ export class ExtensionLoader extends Singleton {
   }
 
   loadOnMain() {
-    registries.MenuRegistry.createInstance();
+    this.autoInitExtensions(() => Promise.resolve([]));
-
-    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;
-    });
   }
 
   loadOnClusterManagerRenderer() {

src/extensions/extensions.injectable.ts

@@ -0,0 +1,42 @@
+/**
+ * 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 { ExtensionLoader } from "./extension-loader";
+import type { LensExtension } from "./lens-extension";
+
+const extensionsInjectable: Injectable<
+  IComputedValue<LensExtension[]>,
+  { extensionLoader: ExtensionLoader }
+> = {
+  getDependencies: () => ({
+    extensionLoader: ExtensionLoader.createInstance(),
+  }),
+
+  lifecycle: lifecycleEnum.singleton,
+
+  instantiate: ({ extensionLoader }) =>
+    computed(() =>
+      [...extensionLoader.instances.values()],
+    ),
+};
+
+export default extensionsInjectable;

src/extensions/lens-main-extension.ts

@@ -24,7 +24,7 @@ import { WindowManager } from "../main/window-manager";
 import { catalogEntityRegistry } from "../main/catalog";
 import type { CatalogEntity } from "../common/catalog";
 import type { IObservableArray } from "mobx";
-import type { MenuRegistration } from "./registries";
+import type { MenuRegistration } from "../main/menu/menu-registration";
 
 export class LensMainExtension extends LensExtension {
   appMenus: MenuRegistration[] = [];

src/main/getDi.ts

@@ -0,0 +1,34 @@
+/**
+ * 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 { createContainer } from "@ogre-tools/injectable";
+
+export const getDi = () =>
+  createContainer(
+    getRequireContextForMainCode,
+    getRequireContextForCommonExtensionCode,
+  );
+
+const getRequireContextForMainCode = () =>
+  require.context("./", true, /\.injectable\.(ts|tsx)$/);
+
+const getRequireContextForCommonExtensionCode = () =>
+  require.context("../extensions", true, /\.injectable\.(ts|tsx)$/);

src/main/getDiForUnitTesting.ts

@@ -0,0 +1,59 @@
+/**
+ * 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 glob from "glob";
+import { memoize } from "lodash/fp";
+
+import {
+  createContainer,
+  ConfigurableDependencyInjectionContainer,
+} from "@ogre-tools/injectable";
+
+export const getDiForUnitTesting = () => {
+  const di: ConfigurableDependencyInjectionContainer = createContainer();
+
+  getInjectableFilePaths()
+    .map(key => {
+      // eslint-disable-next-line @typescript-eslint/no-var-requires
+      const injectable = require(key).default;
+
+      if (!injectable) {
+        console.log(key);
+      }
+
+      return {
+        id: key,
+        ...injectable,
+        aliases: [injectable, ...(injectable.aliases || [])],
+      };
+    })
+
+    .forEach(injectable => di.register(injectable));
+
+  di.preventSideEffects();
+
+  return di;
+};
+
+const getInjectableFilePaths = memoize(() => [
+  ...glob.sync("./**/*.injectable.{ts,tsx}", { cwd: __dirname }),
+  ...glob.sync("../extensions/**/*.injectable.{ts,tsx}", { cwd: __dirname }),
+]);

src/main/index.ts

@@ -61,11 +61,15 @@ import { FilesystemProvisionerStore } from "./extension-filesystem";
 import { SentryInit } from "../common/sentry";
 import { ensureDir } from "fs-extra";
 import { Router } from "./router";
-import { initMenu } from "./menu";
+import { initMenu } from "./menu/menu";
 import { initTray } from "./tray";
 import { kubeApiRequest, shellApiRequest, ShellRequestAuthenticator } from "./proxy-functions";
 import { AppPaths } from "../common/app-paths";
 import { ShellSession } from "./shell-session/shell-session";
+import { getDi } from "./getDi";
+import electronMenuItemsInjectable from "./menu/electron-menu-items.injectable";
+
+const di = getDi();
 
 injectSystemCAs();
 
@@ -236,8 +240,10 @@ app.on("ready", async () => {
   logger.info("🖥️  Starting WindowManager");
   const windowManager = WindowManager.createInstance();
 
+  const menuItems = di.inject(electronMenuItemsInjectable);
+
   onQuitCleanup.push(
-    initMenu(windowManager),
+    initMenu(windowManager, menuItems),
     initTray(windowManager),
     () => ShellSession.cleanup(),
   );

src/main/menu/electron-menu-items.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 { LensMainExtension } from "../../extensions/lens-main-extension";
+import extensionsInjectable from "../../extensions/extensions.injectable";
+import type { MenuRegistration } from "./menu-registration";
+
+const electronMenuItemsInjectable: Injectable<
+  IComputedValue<MenuRegistration[]>,
+  { extensions: IComputedValue<LensMainExtension[]> }
+> = {
+  lifecycle: lifecycleEnum.singleton,
+
+  getDependencies: di => ({
+    extensions: di.inject(extensionsInjectable),
+  }),
+
+  instantiate: ({ extensions }) =>
+    computed(() => extensions.get().flatMap(extension => extension.appMenus)),
+};
+
+export default electronMenuItemsInjectable;

src/main/menu/electron-menu-items.test.ts

@@ -0,0 +1,132 @@
+/**
+ * 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 { ConfigurableDependencyInjectionContainer } from "@ogre-tools/injectable";
+import { LensMainExtension } from "../../extensions/lens-main-extension";
+import electronMenuItemsInjectable from "./electron-menu-items.injectable";
+import type { IComputedValue } from "mobx";
+import { computed, ObservableMap, runInAction } from "mobx";
+import type { MenuRegistration } from "./menu-registration";
+import extensionsInjectable from "../../extensions/extensions.injectable";
+import { getDiForUnitTesting } from "../getDiForUnitTesting";
+
+describe("electron-menu-items", () => {
+  let di: ConfigurableDependencyInjectionContainer;
+  let electronMenuItems: IComputedValue<MenuRegistration[]>;
+  let extensionsStub: ObservableMap<string, LensMainExtension>;
+
+  beforeEach(() => {
+    di = getDiForUnitTesting();
+
+    extensionsStub = new ObservableMap();
+
+    di.override(
+      extensionsInjectable,
+      computed(() => [...extensionsStub.values()]),
+    );
+
+    electronMenuItems = di.inject(electronMenuItemsInjectable);
+  });
+
+  it("does not have any items yet", () => {
+    expect(electronMenuItems.get()).toHaveLength(0);
+  });
+
+  describe("when extension is enabled", () => {
+    beforeEach(() => {
+      const someExtension = new SomeTestExtension({
+        id: "some-extension-id",
+        appMenus: [{ parentId: "some-parent-id-from-first-extension" }],
+      });
+
+      runInAction(() => {
+        extensionsStub.set("some-extension-id", someExtension);
+      });
+    });
+
+    it("has menu items", () => {
+      expect(electronMenuItems.get()).toEqual([
+        {
+          parentId: "some-parent-id-from-first-extension",
+        },
+      ]);
+    });
+
+    it("when disabling extension, does not have menu items", () => {
+      extensionsStub.delete("some-extension-id");
+
+      expect(electronMenuItems.get()).toHaveLength(0);
+    });
+
+    describe("when other extension is enabled", () => {
+      beforeEach(() => {
+        const someOtherExtension = new SomeTestExtension({
+          id: "some-extension-id",
+          appMenus: [{ parentId: "some-parent-id-from-second-extension" }],
+        });
+
+        extensionsStub.set("some-other-extension-id", someOtherExtension);
+      });
+
+      it("has menu items for both extensions", () => {
+        expect(electronMenuItems.get()).toEqual([
+          {
+            parentId: "some-parent-id-from-first-extension",
+          },
+
+          {
+            parentId: "some-parent-id-from-second-extension",
+          },
+        ]);
+      });
+
+      it("when extension is disabled, still returns menu items for extensions that are enabled", () => {
+        runInAction(() => {
+          extensionsStub.delete("some-other-extension-id");
+        });
+
+        expect(electronMenuItems.get()).toEqual([
+          {
+            parentId: "some-parent-id-from-first-extension",
+          },
+        ]);
+      });
+    });
+  });
+});
+
+class SomeTestExtension extends LensMainExtension {
+  constructor({ id, appMenus }: {
+    id: string;
+    appMenus: MenuRegistration[];
+  }) {
+    super({
+      id,
+      absolutePath: "irrelevant",
+      isBundled: false,
+      isCompatible: false,
+      isEnabled: false,
+      manifest: { name: id, version: "some-version" },
+      manifestPath: "irrelevant",
+    });
+
+    this.appMenus = appMenus;
+  }
+}

src/main/menu/menu-registration.d.ts

@@ -0,0 +1,25 @@
+/**
+ * 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 { MenuItemConstructorOptions } from "electron";
+
+export interface MenuRegistration extends MenuItemConstructorOptions {
+  parentId: string;
+}

src/main/menu/menu.ts

@@ -18,18 +18,17 @@
  * 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 { app, BrowserWindow, dialog, Menu, MenuItem, MenuItemConstructorOptions, webContents, shell } from "electron";
-import { autorun } from "mobx";
+import { autorun, IComputedValue } from "mobx";
-import type { WindowManager } from "./window-manager";
+import type { WindowManager } from "../window-manager";
-import { appName, isMac, isWindows, docsUrl, supportUrl, productName } from "../common/vars";
+import { appName, isMac, isWindows, docsUrl, supportUrl, productName } from "../../common/vars";
-import { MenuRegistry } from "../extensions/registries/menu-registry";
+import logger from "../logger";
-import logger from "./logger";
+import { exitApp } from "../exit-app";
-import { exitApp } from "./exit-app";
+import { broadcastMessage } from "../../common/ipc";
-import { broadcastMessage } from "../common/ipc";
+import * as packageJson from "../../../package.json";
-import * as packageJson from "../../package.json";
+import { preferencesURL, extensionsURL, addClusterURL, catalogURL, welcomeURL } from "../../common/routes";
-import { preferencesURL, extensionsURL, addClusterURL, catalogURL, welcomeURL } from "../common/routes";
+import { checkForUpdates, isAutoUpdateEnabled } from "../app-updater";
-import { checkForUpdates, isAutoUpdateEnabled } from "./app-updater";
+import type { MenuRegistration } from "./menu-registration";
 
 export type MenuTopId = "mac" | "file" | "edit" | "view" | "help";
 
@@ -37,8 +36,11 @@ interface MenuItemsOpts extends MenuItemConstructorOptions {
   submenu?: MenuItemConstructorOptions[];
 }
 
-export function initMenu(windowManager: WindowManager) {
+export function initMenu(
-  return autorun(() => buildMenu(windowManager), {
+  windowManager: WindowManager,
+  electronMenuItems: IComputedValue<MenuRegistration[]>,
+) {
+  return autorun(() => buildMenu(windowManager, electronMenuItems.get()), {
     delay: 100,
   });
 }
@@ -61,7 +63,10 @@ export function showAbout(browserWindow: BrowserWindow) {
   });
 }
 
-export function buildMenu(windowManager: WindowManager) {
+export function buildMenu(
+  windowManager: WindowManager,
+  electronMenuItems: MenuRegistration[],
+) {
   function ignoreIf(check: boolean, menuItems: MenuItemConstructorOptions[]) {
     return check ? [] : menuItems;
   }
@@ -302,14 +307,17 @@ export function buildMenu(windowManager: WindowManager) {
   ]);
 
   // Modify menu from extensions-api
-  for (const { parentId, ...menuItem } of MenuRegistry.getInstance().getItems()) {
+  for (const menuItem of electronMenuItems) {
-    if (!appMenu.has(parentId)) {
+    if (!appMenu.has(menuItem.parentId)) {
-      logger.error(`[MENU]: cannot register menu item for parentId=${parentId}, parent item doesn't exist`, { menuItem });
+      logger.error(
+        `[MENU]: cannot register menu item for parentId=${menuItem.parentId}, parent item doesn't exist`,
+        { menuItem },
+      );
 
       continue;
     }
 
-    appMenu.get(parentId).submenu.push(menuItem);
+    appMenu.get(menuItem.parentId).submenu.push(menuItem);
   }
 
   if (!isMac) {

src/main/tray.ts

@@ -23,7 +23,7 @@ import path from "path";
 import packageInfo from "../../package.json";
 import { Menu, Tray } from "electron";
 import { autorun } from "mobx";
-import { showAbout } from "./menu";
+import { showAbout } from "./menu/menu";
 import { checkForUpdates, isAutoUpdateEnabled } from "./app-updater";
 import type { WindowManager } from "./window-manager";
 import logger from "./logger";