Remove extension when folder is removed during runtime (#1518)

Signed-off-by: Panu Horsmalahti <phorsmalahti@mirantis.com>

package.json

@@ -361,6 +361,7 @@
     "nodemon": "^2.0.4",
     "patch-package": "^6.2.2",
     "postinstall-postinstall": "^2.1.0",
+    "prettier": "^2.2.0",
     "progress-bar-webpack-plugin": "^2.1.0",
     "raw-loader": "^4.0.1",
     "react": "^16.14.0",

src/common/utils/rectify-array.ts

@@ -3,6 +3,6 @@
  * @param items either one item or an array of items
  * @returns a list of items
  */
-export function recitfy<T>(items: T | T[]): T[] {
+export function rectify<T>(items: T | T[]): T[] {
   return Array.isArray(items) ? items : [items];
 }

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

@@ -0,0 +1,120 @@
+import { ExtensionLoader } from "../extension-loader";
+
+const manifestPath = "manifest/path";
+const manifestPath2 = "manifest/path2";
+const manifestPath3 = "manifest/path3";
+
+jest.mock(
+  "electron",
+  () => ({
+    ipcRenderer: {
+      invoke: jest.fn(async (channel: string, ...args: any[]) => {
+        if (channel === "extensions:loaded") {
+          return [
+            [
+              manifestPath,
+              {
+                manifest: {
+                  name: "TestExtension",
+                  version: "1.0.0",
+                },
+                manifestPath,
+                isBundled: false,
+                isEnabled: true,
+              },
+            ],
+            [
+              manifestPath2,
+              {
+                manifest: {
+                  name: "TestExtension2",
+                  version: "2.0.0",
+                },
+                manifestPath: manifestPath2,
+                isBundled: false,
+                isEnabled: true,
+              },
+            ],
+          ];
+        }
+      }),
+      on: jest.fn(
+        (channel: string, listener: (event: any, ...args: any[]) => void) => {
+          if (channel === "extensions:loaded") {
+            // First initialize with extensions 1 and 2
+            // and then broadcast event to remove extensioin 2 and add extension number 3
+            setTimeout(() => {
+              listener({}, [
+                [
+                  manifestPath,
+                  {
+                    manifest: {
+                      name: "TestExtension",
+                      version: "1.0.0",
+                    },
+                    manifestPath,
+                    isBundled: false,
+                    isEnabled: true,
+                  },
+                ],
+                [
+                  manifestPath3,
+                  {
+                    manifest: {
+                      name: "TestExtension3",
+                      version: "3.0.0",
+                    },
+                    manifestPath3,
+                    isBundled: false,
+                    isEnabled: true,
+                  },
+                ],
+              ]);
+            }, 10);
+          }
+        }
+      ),
+    },
+  }),
+  {
+    virtual: true,
+  }
+);
+
+describe("ExtensionLoader", () => {
+  it("renderer updates extension after ipc broadcast", async (done) => {
+    const extensionLoader = new ExtensionLoader();
+
+    expect(extensionLoader.userExtensions).toMatchInlineSnapshot(`Map {}`);
+
+    await extensionLoader.init();
+
+    setTimeout(() => {
+      // Assert the extensions after the extension broadcast event
+      expect(extensionLoader.userExtensions).toMatchInlineSnapshot(`
+        Map {
+          "manifest/path" => Object {
+            "isBundled": false,
+            "isEnabled": true,
+            "manifest": Object {
+              "name": "TestExtension",
+              "version": "1.0.0",
+            },
+            "manifestPath": "manifest/path",
+          },
+          "manifest/path3" => Object {
+            "isBundled": false,
+            "isEnabled": true,
+            "manifest": Object {
+              "name": "TestExtension3",
+              "version": "3.0.0",
+            },
+            "manifestPath3": "manifest/path3",
+          },
+        }
+      `);
+
+      done();
+    }, 10);
+  });
+});

src/extensions/extension-loader.ts

@@ -1,4 +1,5 @@
 import { app, ipcRenderer, remote } from "electron";
+import { EventEmitter } from "events";
 import { action, computed, observable, reaction, toJS, when } from "mobx";
 import path from "path";
 import { getHostedCluster } from "../common/cluster-store";
@@ -26,26 +27,32 @@ export class ExtensionLoader {
   protected instances = observable.map<LensExtensionId, LensExtension>();
   protected readonly requestExtensionsChannel = "extensions:loaded";
 
+  // emits event "remove" of type LensExtension when the extension is removed
+  private events = new EventEmitter();
+
   @observable isLoaded = false;
   whenLoaded = when(() => this.isLoaded);
 
   @computed get userExtensions(): Map<LensExtensionId, InstalledExtension> {
     const extensions = this.extensions.toJS();
+
     extensions.forEach((ext, extId) => {
       if (ext.isBundled) {
         extensions.delete(extId);
       }
     });
+
     return extensions;
   }
 
   @action
   async init() {
     if (ipcRenderer) {
-      this.initRenderer();
+      await this.initRenderer();
     } else {
-      this.initMain();
+      await this.initMain();
     }
+
     extensionsStore.manageState(this);
   }
 
@@ -57,11 +64,28 @@ export class ExtensionLoader {
     this.extensions.set(extension.manifestPath as LensExtensionId, extension);
   }
 
+  removeInstance(lensExtensionId: LensExtensionId) {
+    logger.info(`${logModule} deleting extension instance ${lensExtensionId}`);
+    const instance = this.instances.get(lensExtensionId);
+
+    if (instance) {
+      try {
+        instance.disable();
+        this.events.emit("remove", instance);
+        this.instances.delete(lensExtensionId);
+      } catch (error) {
+        logger.error(`${logModule}: deactivation extension error`, { lensExtensionId, error });
+      }
+    }
+  }
+
   removeExtension(lensExtensionId: LensExtensionId) {
-    // TODO: Remove the extension properly (from menus etc.)
+    this.removeInstance(lensExtensionId);
+
     if (!this.extensions.delete(lensExtensionId)) {
       throw new Error(`Can't remove extension ${lensExtensionId}, doesn't exist.`);
     }
+
   }
 
   protected async initMain() {
@@ -79,14 +103,25 @@ export class ExtensionLoader {
   }
 
   protected async initRenderer() {
-    const extensionListHandler = ( extensions: [LensExtensionId, InstalledExtension][]) => {
+    const extensionListHandler = (extensions: [LensExtensionId, InstalledExtension][]) => {
       this.isLoaded = true;
+      const receivedExtensionIds = extensions.map(([lensExtensionId]) => lensExtensionId);
+
+      // Add new extensions
       extensions.forEach(([extId, ext]) => {
         if (!this.extensions.has(extId)) {
           this.extensions.set(extId, ext);
         }
       });
+
+      // Remove deleted extensions
+      this.extensions.forEach((_, lensExtensionId) => {
+        if (!receivedExtensionIds.includes(lensExtensionId)) {
+          this.removeExtension(lensExtensionId);
+        }
+      });
     };
+
     requestMain(this.requestExtensionsChannel).then(extensionListHandler);
     subscribeToBroadcast(this.requestExtensionsChannel, (event, extensions: [LensExtensionId, InstalledExtension][]) => {
       extensionListHandler(extensions);
@@ -95,36 +130,73 @@ export class ExtensionLoader {
 
   loadOnMain() {
     logger.info(`${logModule}: load on main`);
-    this.autoInitExtensions(async (ext: LensMainExtension) => [
+    this.autoInitExtensions(async (extension: LensMainExtension) => {
-      registries.menuRegistry.add(ext.appMenus)
+      // Each .add returns a function to remove the item
-    ]);
+      const removeItems = [
+        registries.menuRegistry.add(extension.appMenus)
+      ];
+
+      this.events.on("remove", (removedExtension: LensRendererExtension) => {
+        // manifestPath is considered the id
+        if (removedExtension.manifestPath === extension.manifestPath) {
+          removeItems.forEach(remove => {
+            remove();
+          });
+        }
+      });
+
+      return removeItems;
+    });
   }
 
   loadOnClusterManagerRenderer() {
     logger.info(`${logModule}: load on main renderer (cluster manager)`);
-    this.autoInitExtensions(async (ext: LensRendererExtension) => [
+    this.autoInitExtensions(async (extension: LensRendererExtension) => {
-      registries.globalPageRegistry.add(ext.globalPages, ext),
+      const removeItems = [
-      registries.globalPageMenuRegistry.add(ext.globalPageMenus, ext),
+        registries.globalPageRegistry.add(extension.globalPages, extension),
-      registries.appPreferenceRegistry.add(ext.appPreferences),
+        registries.globalPageMenuRegistry.add(extension.globalPageMenus, extension),
-      registries.clusterFeatureRegistry.add(ext.clusterFeatures),
+        registries.appPreferenceRegistry.add(extension.appPreferences),
-      registries.statusBarRegistry.add(ext.statusBarItems),
+        registries.clusterFeatureRegistry.add(extension.clusterFeatures),
-    ]);
+        registries.statusBarRegistry.add(extension.statusBarItems),
+      ];
+
+      this.events.on("remove", (removedExtension: LensRendererExtension) => {
+        if (removedExtension.manifestPath === extension.manifestPath) {
+          removeItems.forEach(remove => {
+            remove();
+          });
+        }
+      });
+
+      return removeItems;
+    });
   }
 
   loadOnClusterRenderer() {
     logger.info(`${logModule}: load on cluster renderer (dashboard)`);
     const cluster = getHostedCluster();
-    this.autoInitExtensions(async (ext: LensRendererExtension) => {
+    this.autoInitExtensions(async (extension: LensRendererExtension) => {
-      if (await ext.isEnabledForCluster(cluster) === false) {
+      if (await extension.isEnabledForCluster(cluster) === false) {
         return [];
       }
-      return [
+
-        registries.clusterPageRegistry.add(ext.clusterPages, ext),
+      const removeItems = [
-        registries.clusterPageMenuRegistry.add(ext.clusterPageMenus, ext),
+        registries.clusterPageRegistry.add(extension.clusterPages, extension),
-        registries.kubeObjectMenuRegistry.add(ext.kubeObjectMenuItems),
+        registries.clusterPageMenuRegistry.add(extension.clusterPageMenus, extension),
-        registries.kubeObjectDetailRegistry.add(ext.kubeObjectDetailItems),
+        registries.kubeObjectMenuRegistry.add(extension.kubeObjectMenuItems),
-        registries.kubeObjectStatusRegistry.add(ext.kubeObjectStatusTexts)
+        registries.kubeObjectDetailRegistry.add(extension.kubeObjectDetailItems),
+        registries.kubeObjectStatusRegistry.add(extension.kubeObjectStatusTexts)
       ];
+
+      this.events.on("remove", (removedExtension: LensRendererExtension) => {
+        if (removedExtension.manifestPath === extension.manifestPath) {
+          removeItems.forEach(remove => {
+            remove();
+          });
+        }
+      });
+
+      return removeItems;
     });
   }
 
@@ -148,13 +220,7 @@ export class ExtensionLoader {
             logger.error(`${logModule}: activation extension error`, { ext, err });
           }
         } else if (!ext.isEnabled && alreadyInit) {
-          try {
+          this.removeInstance(extId);
-            const instance = this.instances.get(extId);
-            instance.disable();
-            this.instances.delete(extId);
-          } catch (err) {
-            logger.error(`${logModule}: deactivation extension error`, { ext, err });
-          }
         }
       }
     }, {

src/extensions/registries/base-registry.ts

@@ -1,7 +1,7 @@
 // Base class for extensions-api registries
 import { action, observable } from "mobx";
 import { LensExtension } from "../lens-extension";
-import { recitfy } from "../../common/utils";
+import { rectify } from "../../common/utils";
 
 export class BaseRegistry<T> {
   private items = observable<T>([], { deep: false });
@@ -13,7 +13,7 @@ export class BaseRegistry<T> {
   add(items: T | T[], ext?: LensExtension): () => void; // allow method overloading with required "ext"
   @action
   add(items: T | T[]) {
-    const itemArray = recitfy(items);
+    const itemArray = rectify(items);
     this.items.push(...itemArray);
     return () => this.remove(...itemArray);
   }

src/extensions/registries/page-registry.ts

@@ -7,7 +7,7 @@ import { compile } from "path-to-regexp";
 import { BaseRegistry } from "./base-registry";
 import { LensExtension, sanitizeExtensionName } from "../lens-extension";
 import logger from "../../main/logger";
-import { recitfy } from "../../common/utils";
+import { rectify } from "../../common/utils";
 
 export interface PageRegistration {
   /**
@@ -54,7 +54,7 @@ export function getExtensionPageUrl<P extends object>({ extensionId, pageId = ""
 export class PageRegistry extends BaseRegistry<RegisteredPage> {
   @action
   add(items: PageRegistration | PageRegistration[], ext: LensExtension) {
-    const itemArray = recitfy(items);
+    const itemArray = rectify(items);
     let registeredPages: RegisteredPage[] = [];
     try {
       registeredPages = itemArray.map(page => ({

src/renderer/components/+extensions/extensions.tsx

@@ -277,6 +277,7 @@ export class Extensions extends React.Component {
 
   renderExtensions() {
     const { extensions, extensionsPath, search } = this;
+
     if (!extensions.length) {
       return (
         <div className="no-extensions flex box gaps justify-center">
@@ -288,6 +289,7 @@ export class Extensions extends React.Component {
         </div>
       );
     }
+
     return extensions.map(ext => {
       const { manifestPath: extId, isEnabled, manifest } = ext;
       const { name, description } = manifest;

yarn.lock

@@ -11460,6 +11460,11 @@ prepend-http@^2.0.0:
   resolved "https://registry.yarnpkg.com/prepend-http/-/prepend-http-2.0.0.tgz#e92434bfa5ea8c19f41cdfd401d741a3c819d897"
   integrity sha1-6SQ0v6XqjBn0HN/UAddBo8gZ2Jc=
 
+prettier@^2.2.0:
+  version "2.2.0"
+  resolved "https://registry.yarnpkg.com/prettier/-/prettier-2.2.0.tgz#8a03c7777883b29b37fb2c4348c66a78e980418b"
+  integrity sha512-yYerpkvseM4iKD/BXLYUkQV5aKt4tQPqaGW6EsZjzyu0r7sVZZNPJW4Y8MyKmicp6t42XUPcBVA+H6sB3gqndw==
+
 pretty-error@^2.1.1:
   version "2.1.1"
   resolved "https://registry.yarnpkg.com/pretty-error/-/pretty-error-2.1.1.tgz#5f4f87c8f91e5ae3f3ba87ab4cf5e03b1a17f1a3"