Add behaviour for tray menu items originating from extensions (#5609)

src/behaviours/cluster/sidebar-and-tab-navigation-for-extensions.test.tsx

@@ -46,7 +46,7 @@ describe("cluster - sidebar and tab navigation for extensions", () => {
       const getRendererExtensionFake = getRendererExtensionFakeFor(applicationBuilder);
       const testExtension = getRendererExtensionFake(extensionStubWithSidebarItems);
 
-      await applicationBuilder.addExtensions(testExtension);
+      await applicationBuilder.extensions.renderer.enable(testExtension);
     });
 
     describe("given no state for expanded sidebar items exists, and navigated to child sidebar item, when rendered", () => {

src/behaviours/extension-special-characters-in-page-registrations.test.tsx

@@ -23,7 +23,7 @@ describe("extension special characters in page registrations", () => {
       extensionWithPagesHavingSpecialCharacters,
     );
 
-    await applicationBuilder.addExtensions(testExtension);
+    await applicationBuilder.extensions.renderer.enable(testExtension);
 
     rendered = await applicationBuilder.render();
   });

src/behaviours/navigate-to-extension-page.test.tsx

@@ -27,7 +27,7 @@ describe("navigate to extension page", () => {
       extensionWithPagesHavingParameters,
     );
 
-    await applicationBuilder.addExtensions(testExtension);
+    await applicationBuilder.extensions.renderer.enable(testExtension);
 
     rendered = await applicationBuilder.render();
 

src/behaviours/preferences/navigation-to-extension-specific-preferences.test.tsx

@@ -44,11 +44,11 @@ describe("preferences - navigation to extension specific preferences", () => {
     });
 
     describe("when extension with specific preferences is enabled", () => {
-      beforeEach(() => {
+      beforeEach(async () => {
         const getRendererExtensionFake = getRendererExtensionFakeFor(applicationBuilder);
         const testExtension = getRendererExtensionFake(extensionStubWithExtensionSpecificPreferenceItems);
 
-        applicationBuilder.addExtensions(testExtension);
+        await applicationBuilder.extensions.renderer.enable(testExtension);
       });
 
       it("renders", () => {

src/behaviours/preferences/navigation-to-telemetry-preferences.test.tsx

@@ -50,7 +50,7 @@ describe("preferences - navigation to telemetry preferences", () => {
         const getRendererExtensionFake = getRendererExtensionFakeFor(applicationBuilder);
         const testExtensionWithTelemetryPreferenceItems = getRendererExtensionFake(extensionStubWithTelemetryPreferenceItems);
 
-        applicationBuilder.addExtensions(
+        applicationBuilder.extensions.renderer.enable(
           testExtensionWithTelemetryPreferenceItems,
         );
       });
@@ -105,7 +105,7 @@ describe("preferences - navigation to telemetry preferences", () => {
         ],
       });
 
-      applicationBuilder.addExtensions(
+      applicationBuilder.extensions.renderer.enable(
         testExtensionWithTelemetryPreferenceItems,
       );
 

src/behaviours/tray/clicking-tray-menu-item-originating-from-extension.test.ts

@@ -0,0 +1,134 @@
+/**
+ * Copyright (c) OpenLens Authors. All rights reserved.
+ * Licensed under MIT License. See LICENSE in root directory for more information.
+ */
+import { LensMainExtension } from "../../extensions/lens-main-extension";
+import type { TrayMenuRegistration } from "../../main/tray/tray-menu-registration";
+import type { ApplicationBuilder } from "../../renderer/components/test-utils/get-application-builder";
+import { getApplicationBuilder } from "../../renderer/components/test-utils/get-application-builder";
+import loggerInjectable from "../../common/logger.injectable";
+import type { Logger } from "../../common/logger";
+
+describe("clicking tray menu item originating from extension", () => {
+  let applicationBuilder: ApplicationBuilder;
+  let logErrorMock: jest.Mock;
+
+  beforeEach(async () => {
+    applicationBuilder = getApplicationBuilder();
+
+    applicationBuilder.beforeApplicationStart(({ mainDi }) => {
+      logErrorMock = jest.fn();
+
+      mainDi.override(loggerInjectable, () => ({ error: logErrorMock }) as unknown as Logger);
+    });
+
+    await applicationBuilder.render();
+  });
+
+  describe("when extension is enabled", () => {
+    let someExtension: SomeTestExtension;
+    let clickMock: jest.Mock;
+
+    beforeEach(async () => {
+      clickMock = jest.fn();
+
+      someExtension = new SomeTestExtension({
+        id: "some-extension-id",
+        trayMenus: [{ label: "some-label", click: clickMock }],
+      });
+
+      await applicationBuilder.extensions.main.enable(someExtension);
+    });
+
+    it("when item is clicked, triggers the click handler", () => {
+      applicationBuilder.tray.click(
+        "some-label-tray-menu-item-for-extension-some-extension-id-instance-1",
+      );
+
+      expect(clickMock).toHaveBeenCalled();
+    });
+
+    describe("given click handler throws synchronously, when item is clicked", () => {
+      beforeEach(() => {
+        clickMock.mockImplementation(() => {
+          throw new Error("some-error");
+        });
+
+        applicationBuilder.tray.click(
+          "some-label-tray-menu-item-for-extension-some-extension-id-instance-1",
+        );
+      });
+
+      it("logs the error", () => {
+        expect(logErrorMock).toHaveBeenCalledWith(
+          '[TRAY]: Clicking of tray item "some-label" from extension "some-extension-id" failed.',
+          expect.any(Error),
+        );
+      });
+    });
+
+    describe("given click handler rejects asynchronously, when item is clicked", () => {
+      beforeEach(() => {
+        clickMock.mockImplementation(() => Promise.reject("some-rejection"));
+
+        applicationBuilder.tray.click(
+          "some-label-tray-menu-item-for-extension-some-extension-id-instance-1",
+        );
+      });
+
+      it("logs the error", () => {
+        expect(logErrorMock).toHaveBeenCalledWith(
+          '[TRAY]: Clicking of tray item "some-label" from extension "some-extension-id" failed.',
+          "some-rejection",
+        );
+      });
+    });
+
+    describe("when extension is disabled", () => {
+      beforeEach(() => {
+        applicationBuilder.extensions.main.disable(someExtension);
+      });
+
+      it("does not have the tray menu item from extension", () => {
+        applicationBuilder.extensions.main.disable(someExtension);
+
+        expect(
+          applicationBuilder.tray.get(
+            "some-label-tray-menu-item-for-extension-some-extension-id-instance-1",
+          ),
+        ).toBeNull();
+      });
+
+      // Note: Motivation here is to make sure that enabling same extension does not throw
+      it("when extension is re-enabled, has the tray menu item from extension", async () => {
+        await applicationBuilder.extensions.main.enable(someExtension);
+
+        expect(
+          applicationBuilder.tray.get(
+            "some-label-tray-menu-item-for-extension-some-extension-id-instance-2",
+          ),
+        ).not.toBeNull();
+      });
+    });
+
+  });
+});
+
+class SomeTestExtension extends LensMainExtension {
+  constructor({ id, trayMenus }: {
+     id: string;
+     trayMenus: TrayMenuRegistration[];
+   }) {
+    super({
+      id,
+      absolutePath: "irrelevant",
+      isBundled: false,
+      isCompatible: false,
+      isEnabled: false,
+      manifest: { name: id, version: "some-version", engines: { lens: "^5.5.0" }},
+      manifestPath: "irrelevant",
+    });
+
+    this.trayMenus = trayMenus;
+  }
+}

src/main/tray/tray-menu-item/tray-menu-item-registrator.injectable.ts

@@ -55,20 +55,27 @@ const toItemInjectablesFor = (extension: LensMainExtension, installationCounter:
         label: computed(() => registration.label || ""),
         tooltip: registration.toolTip,
 
-        click: pipeline(
+        click: () => {
-          () => {
+          const decorated = pipeline(
-            registration.click?.(registration);
+            registration.click || (() => {}),
-          },
 
-          withErrorLoggingFor(() => `[TRAY]: Clicking of tray item "${trayItemId}" from extension "${extension.sanitizedExtensionId}" failed.`),
+            withErrorLoggingFor(
+              () =>
+                `[TRAY]: Clicking of tray item "${trayItemId}" from extension "${extension.sanitizedExtensionId}" failed.`,
+            ),
 
-          // TODO: Find out how to improve typing so that instead of
+            // TODO: Find out how to improve typing so that instead of
-          // x => withErrorSuppression(x) there could only be withErrorSuppression
+            // x => withErrorSuppression(x) there could only be withErrorSuppression
-          (x) => withErrorSuppression(x),
+            x => withErrorSuppression(x),
-        ),
+          );
+
+          return decorated(registration);
+        },
 
         enabled: computed(() => !!registration.enabled),
         visible: computed(() => true),
+
+        extension,
       }),
 
       injectionToken: trayMenuItemInjectionToken,

src/main/tray/tray-menu-items.injectable.ts

@@ -1,19 +0,0 @@
-/**
- * Copyright (c) OpenLens Authors. All rights reserved.
- * Licensed under MIT License. See LICENSE in root directory for more information.
- */
-import { getInjectable } from "@ogre-tools/injectable";
-import { computed } from "mobx";
-import mainExtensionsInjectable from "../../extensions/main-extensions.injectable";
-
-const trayItemsInjectable = getInjectable({
-  id: "tray-items",
-
-  instantiate: (di) => {
-    const extensions = di.inject(mainExtensionsInjectable);
-
-    return computed(() => extensions.get().flatMap(extension => extension.trayMenus));
-  },
-});
-
-export default trayItemsInjectable;

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

@@ -1,120 +0,0 @@
-/**
- * Copyright (c) OpenLens Authors. All rights reserved.
- * Licensed under MIT License. See LICENSE in root directory for more information.
- */
-import type { DiContainer } from "@ogre-tools/injectable";
-import { LensMainExtension } from "../../extensions/lens-main-extension";
-import trayItemsInjectable from "./tray-menu-items.injectable";
-import type { IComputedValue } from "mobx";
-import { computed, ObservableMap, runInAction } from "mobx";
-import { getDiForUnitTesting } from "../getDiForUnitTesting";
-import mainExtensionsInjectable from "../../extensions/main-extensions.injectable";
-import type { TrayMenuRegistration } from "./tray-menu-registration";
-
-describe("tray-menu-items", () => {
-  let di: DiContainer;
-  let trayMenuItems: IComputedValue<TrayMenuRegistration[]>;
-  let extensionsStub: ObservableMap<string, LensMainExtension>;
-
-  beforeEach(async () => {
-    di = getDiForUnitTesting({ doGeneralOverrides: true });
-
-    extensionsStub = new ObservableMap();
-
-    di.override(
-      mainExtensionsInjectable,
-      () => computed(() => [...extensionsStub.values()]),
-    );
-
-    trayMenuItems = di.inject(trayItemsInjectable);
-  });
-
-  it("does not have any items yet", () => {
-    expect(trayMenuItems.get()).toHaveLength(0);
-  });
-
-  describe("when extension is enabled", () => {
-    beforeEach(() => {
-      const someExtension = new SomeTestExtension({
-        id: "some-extension-id",
-        trayMenus: [{ label: "tray-menu-from-some-extension" }],
-      });
-
-      runInAction(() => {
-        extensionsStub.set("some-extension-id", someExtension);
-      });
-    });
-
-    it("has tray menu items", () => {
-      expect(trayMenuItems.get()).toEqual([
-        {
-          label: "tray-menu-from-some-extension",
-        },
-      ]);
-    });
-
-    it("when disabling extension, does not have tray menu items", () => {
-      runInAction(() => {
-        extensionsStub.delete("some-extension-id");
-      });
-
-      expect(trayMenuItems.get()).toHaveLength(0);
-    });
-
-    describe("when other extension is enabled", () => {
-      beforeEach(() => {
-        const someOtherExtension = new SomeTestExtension({
-          id: "some-extension-id",
-          trayMenus: [{ label: "some-label-from-second-extension" }],
-        });
-
-        runInAction(() => {
-          extensionsStub.set("some-other-extension-id", someOtherExtension);
-        });
-      });
-
-      it("has tray menu items for both extensions", () => {
-        expect(trayMenuItems.get()).toEqual([
-          {
-            label: "tray-menu-from-some-extension",
-          },
-
-          {
-            label: "some-label-from-second-extension",
-          },
-        ]);
-      });
-
-      it("when extension is disabled, still returns tray menu items for extensions that are enabled", () => {
-        runInAction(() => {
-          extensionsStub.delete("some-other-extension-id");
-        });
-
-        expect(trayMenuItems.get()).toEqual([
-          {
-            label: "tray-menu-from-some-extension",
-          },
-        ]);
-      });
-    });
-  });
-});
-
-class SomeTestExtension extends LensMainExtension {
-  constructor({ id, trayMenus }: {
-     id: string;
-     trayMenus: TrayMenuRegistration[];
-   }) {
-    super({
-      id,
-      absolutePath: "irrelevant",
-      isBundled: false,
-      isCompatible: false,
-      isEnabled: false,
-      manifest: { name: id, version: "some-version", engines: { lens: "^5.5.0" }},
-      manifestPath: "irrelevant",
-    });
-
-    this.trayMenus = trayMenus;
-  }
-}

src/renderer/components/test-utils/get-application-builder.tsx

@@ -6,7 +6,7 @@ import type { LensRendererExtension } from "../../../extensions/lens-renderer-ex
 import rendererExtensionsInjectable from "../../../extensions/renderer-extensions.injectable";
 import currentlyInClusterFrameInjectable from "../../routes/currently-in-cluster-frame.injectable";
 import { extensionRegistratorInjectionToken } from "../../../extensions/extension-loader/extension-registrator-injection-token";
-import type { IObservableArray } from "mobx";
+import type { IObservableArray, ObservableSet } from "mobx";
 import { computed, observable, runInAction } from "mobx";
 import { renderFor } from "./renderFor";
 import React from "react";
@@ -24,7 +24,7 @@ import type { ClusterStore } from "../../../common/cluster-store/cluster-store";
 import mainExtensionsInjectable from "../../../extensions/main-extensions.injectable";
 import currentRouteComponentInjectable from "../../routes/current-route-component.injectable";
 import { pipeline } from "@ogre-tools/fp";
-import { flatMap, compact, join, get, filter, map, matches, find } from "lodash/fp";
+import { flatMap, compact, join, get, filter, map, matches } from "lodash/fp";
 import preferenceNavigationItemsInjectable from "../+preferences/preferences-navigation/preference-navigation-items.injectable";
 import navigateToPreferencesInjectable from "../../../common/front-end-routing/routes/preferences/navigate-to-preferences.injectable";
 import type { MenuItemOpts } from "../../../main/menu/application-menu-items.injectable";
@@ -56,13 +56,31 @@ import { openMenu } from "react-select-event";
 import userEvent from "@testing-library/user-event";
 import { StatusBar } from "../status-bar/status-bar";
 import lensProxyPortInjectable from "../../../main/lens-proxy/lens-proxy-port.injectable";
+import type { LensMainExtension } from "../../../extensions/lens-main-extension";
+import trayMenuItemsInjectable from "../../../main/tray/tray-menu-item/tray-menu-items.injectable";
+import type { LensExtension } from "../../../extensions/lens-extension";
 
 type Callback = (dis: DiContainers) => void | Promise<void>;
 
+type EnableExtensions<T> = (...extensions: T[]) => Promise<void>;
+type DisableExtensions<T> = (...extensions: T[]) => void;
+
 export interface ApplicationBuilder {
   dis: DiContainers;
   setEnvironmentToClusterFrame: () => ApplicationBuilder;
-  addExtensions: (...extensions: LensRendererExtension[]) => Promise<ApplicationBuilder>;
+
+  extensions: {
+    renderer: {
+      enable: EnableExtensions<LensRendererExtension>;
+      disable: DisableExtensions<LensRendererExtension>;
+    };
+
+    main: {
+      enable: EnableExtensions<LensMainExtension>;
+      disable: DisableExtensions<LensMainExtension>;
+    };
+  };
+
   allowKubeResource: (resourceName: KubeResource) => ApplicationBuilder;
   beforeApplicationStart: (callback: Callback) => ApplicationBuilder;
   beforeRender: (callback: Callback) => ApplicationBuilder;
@@ -135,7 +153,8 @@ export const getApplicationBuilder = () => {
   const beforeApplicationStartCallbacks: Callback[] = [];
   const beforeRenderCallbacks: Callback[] = [];
 
-  const extensionsState = observable.array<LensRendererExtension>();
+  const rendererExtensionsState = observable.set<LensRendererExtension>();
+  const mainExtensionsState = observable.set<LensMainExtension>();
 
   rendererDi.override(subscribeStoresInjectable, () => () => () => {});
 
@@ -174,14 +193,13 @@ export const getApplicationBuilder = () => {
   );
 
   rendererDi.override(rendererExtensionsInjectable, () =>
-    computed(() => extensionsState),
+    computed(() => [...rendererExtensionsState]),
   );
 
   mainDi.override(mainExtensionsInjectable, () =>
-    computed(() => []),
+    computed(() => [...mainExtensionsState]),
   );
 
-  let trayMenuItemsStateFake: TrayMenuItem[];
   let trayMenuIconPath: string;
 
   mainDi.override(electronTrayInjectable, () => ({
@@ -191,9 +209,7 @@ export const getApplicationBuilder = () => {
       trayMenuIconPath = iconPaths.normal;
     },
     stop: () => {},
-    setMenuItems: (items) => {
+    setMenuItems: () => {},
-      trayMenuItemsStateFake = items;
-    },
     setIconPath: (path) => {
       trayMenuIconPath = path;
     },
@@ -202,6 +218,43 @@ export const getApplicationBuilder = () => {
   let allowedResourcesState: IObservableArray<KubeResource>;
   let rendered: RenderResult;
 
+  const enableExtensionsFor = <T extends ObservableSet>(
+    extensionState: T,
+    di: DiContainer,
+  ) => {
+    let index = 0;
+
+    return async (...extensions: LensExtension[]) => {
+      const extensionRegistrators = di.injectMany(
+        extensionRegistratorInjectionToken,
+      );
+
+      const addAndEnableExtensions = async () => {
+        index++;
+
+        const registratorPromises = extensions.flatMap((extension) =>
+          extensionRegistrators.map((registrator) =>
+            registrator(extension, index),
+          ),
+        );
+
+        await Promise.all(registratorPromises);
+
+        runInAction(() => {
+          extensions.forEach((extension) => {
+            extensionState.add(extension);
+          });
+        });
+      };
+
+      if (rendered) {
+        await addAndEnableExtensions();
+      } else {
+        builder.beforeRender(addAndEnableExtensions);
+      }
+    };
+  };
+
   const builder: ApplicationBuilder = {
     dis,
 
@@ -242,18 +295,20 @@ export const getApplicationBuilder = () => {
 
     tray: {
       get: (id: string) => {
-        return trayMenuItemsStateFake.find(matches({ id })) ?? null;
+        const trayMenuItems = mainDi.inject(trayMenuItemsInjectable).get();
+
+        return trayMenuItems.find(matches({ id })) ?? null;
       },
       getIconPath: () => trayMenuIconPath,
+
       click: async (id: string) => {
-        const menuItem = pipeline(
+        const trayMenuItems = mainDi.inject(trayMenuItemsInjectable).get();
-          trayMenuItemsStateFake,
+
-          find((menuItem) => menuItem.id === id),
+        const menuItem = trayMenuItems.find(matches({ id })) ?? null;
-        );
 
         if (!menuItem) {
           const availableIds = pipeline(
-            trayMenuItemsStateFake,
+            trayMenuItems,
             filter(item => !!item.click),
             map(item => item.id),
             join(", "),
@@ -345,32 +400,16 @@ export const getApplicationBuilder = () => {
       return builder;
     },
 
-    addExtensions: async (...extensions) => {
+    extensions: {
-      const extensionRegistrators = rendererDi.injectMany(
+      renderer: {
-        extensionRegistratorInjectionToken,
+        enable: enableExtensionsFor(rendererExtensionsState, rendererDi),
-      );
+        disable: disableExtensionsFor(rendererExtensionsState),
+      },
 
-      const addAndEnableExtensions = async () => {
+      main: {
-        const registratorPromises = extensions.flatMap((extension) =>
+        enable: enableExtensionsFor(mainExtensionsState, mainDi),
-          extensionRegistrators.map((registrator) => registrator(extension, 1)),
+        disable: disableExtensionsFor(mainExtensionsState),
-        );
+      },
-
-        await Promise.all(registratorPromises);
-
-        runInAction(() => {
-          extensions.forEach((extension) => {
-            extensionsState.push(extension);
-          });
-        });
-      };
-
-      if (rendered) {
-        await addAndEnableExtensions();
-      } else {
-        builder.beforeRender(addAndEnableExtensions);
-      }
-
-      return builder;
     },
 
     allowKubeResource: (resourceName) => {
@@ -494,3 +533,14 @@ function toFlatChildren(parentId: string | null | undefined): ToFlatChildren {
     ),
   ];
 }
+
+const disableExtensionsFor =
+    <T extends ObservableSet>(extensionState: T) =>
+
+    (...extensions: LensExtension[]) => {
+      extensions.forEach((extension) => {
+        runInAction(() => {
+          extensionState.delete(extension);
+        });
+      });
+    };