Remove symlink on extension install and manual runtime uninstall (#1718)

* Remove broken symlink on extension install

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

* Remove broken symlink on manual uninstall during runtime

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

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

@@ -18,7 +18,7 @@ const mockedWatch = watch as jest.MockedFunction<typeof watch>;
 
 describe("ExtensionDiscovery", () => {
   it("emits add for added extension", async done => {
-    globalThis.__non_webpack_require__.mockImplementationOnce(() => ({
+    globalThis.__non_webpack_require__.mockImplementation(() => ({
       name: "my-extension"
     }));
     let addHandler: (filePath: string) => void;
@@ -61,9 +61,6 @@ describe("ExtensionDiscovery", () => {
   });
 
   it("doesn't emit add for added file under extension", async done => {
-    globalThis.__non_webpack_require__.mockImplementationOnce(() => ({
-      name: "my-extension"
-    }));
     let addHandler: (filePath: string) => void;
 
     const mockWatchInstance: any = {

src/extensions/extension-discovery.ts

@@ -155,23 +155,26 @@ export class ExtensionDiscovery {
       .on("unlinkDir", this.handleWatchUnlinkDir);
   }
 
-  handleWatchFileAdd =  async (filePath: string) => {
+  handleWatchFileAdd =  async (manifestPath: string) => {
     // e.g. "foo/package.json"
-    const relativePath = path.relative(this.localFolderPath, filePath);
+    const relativePath = path.relative(this.localFolderPath, manifestPath);
 
     // Converts "foo/package.json" to ["foo", "package.json"], where length of 2 implies
     // that the added file is in a folder under local folder path.
     // This safeguards against a file watch being triggered under a sub-directory which is not an extension.
     const isUnderLocalFolderPath = relativePath.split(path.sep).length === 2;
 
-    if (path.basename(filePath) === manifestFilename && isUnderLocalFolderPath) {
+    if (path.basename(manifestPath) === manifestFilename && isUnderLocalFolderPath) {
       try {
-        const absPath = path.dirname(filePath);
+        const absPath = path.dirname(manifestPath);
 
         // this.loadExtensionFromPath updates this.packagesJson
-        const extension = await this.loadExtensionFromPath(absPath);
+        const extension = await this.loadExtensionFromFolder(absPath);
 
         if (extension) {
+          // Remove a broken symlink left by a previous installation if it exists.
+          await this.removeSymlinkByManifestPath(manifestPath);
+
           // Install dependencies for the new extension
           await this.installPackages();
 
@@ -199,6 +202,9 @@ export class ExtensionDiscovery {
         .find(([, extensionFolder]) => filePath === extensionFolder)?.[0];
 
       if (extensionName !== undefined) {
+        // If the extension is deleted manually while the application is running, also remove the symlink
+        await this.removeSymlinkByPackageName(extensionName);
+
         delete this.packagesJson.dependencies[extensionName];
 
         // Reinstall dependencies to remove the extension from package.json
@@ -216,6 +222,26 @@ export class ExtensionDiscovery {
     }
   };
 
+  /**
+   * Remove the symlink under node_modules if exists.
+   * If we don't remove the symlink, the uninstall would leave a non-working symlink,
+   * which wouldn't be fixed if the extension was reinstalled, causing the extension not to work.
+   * @param name e.g. "@mirantis/lens-extension-cc"
+   */
+  removeSymlinkByPackageName(name: string) {
+    return fs.remove(this.getInstalledPath(name));
+  }
+
+  /**
+   * Remove the symlink under node_modules if it exists.
+   * @param manifestPath Path to package.json
+   */
+  removeSymlinkByManifestPath(manifestPath: string) {
+    const manifestJson = __non_webpack_require__(manifestPath);
+
+    return this.removeSymlinkByPackageName(manifestJson.name);
+  }
+
   /**
    * Uninstalls extension.
    * The application will detect the folder unlink and remove the extension from the UI automatically.
@@ -224,16 +250,7 @@ export class ExtensionDiscovery {
   async uninstallExtension({ absolutePath, manifest }: InstalledExtension) {
     logger.info(`${logModule} Uninstalling ${manifest.name}`);
 
-    // remove the symlink under node_modules.
+    await this.removeSymlinkByPackageName(manifest.name);
-    // If we don't remove the symlink, the uninstall would leave a non-working symlink,
-    // which wouldn't be fixed if the extension was reinstalled, causing the extension not to work.
-    await fs.remove(this.getInstalledPath(manifest.name));
-
-    const exists = await fs.pathExists(absolutePath);
-
-    if (!exists) {
-      throw new Error(`Extension path ${absolutePath} doesn't exist`);
-    }
 
     // fs.remove does nothing if the path doesn't exist anymore
     await fs.remove(absolutePath);
@@ -290,6 +307,10 @@ export class ExtensionDiscovery {
     return path.join(this.getInstalledPath(name), manifestFilename);
   }
 
+  /**
+   * Returns InstalledExtension from path to package.json file.
+   * Also updates this.packagesJson.
+   */
   protected async getByManifest(manifestPath: string, { isBundled = false }: {
     isBundled?: boolean;
   } = {}): Promise<InstalledExtension | null> {
@@ -349,7 +370,7 @@ export class ExtensionDiscovery {
       }
 
       const absPath = path.resolve(folderPath, fileName);
-      const extension = await this.loadExtensionFromPath(absPath, { isBundled: true });
+      const extension = await this.loadExtensionFromFolder(absPath, { isBundled: true });
 
       if (extension) {
         extensions.push(extension);
@@ -384,7 +405,7 @@ export class ExtensionDiscovery {
         continue;
       }
 
-      const extension = await this.loadExtensionFromPath(absPath);
+      const extension = await this.loadExtensionFromFolder(absPath);
 
       if (extension) {
         extensions.push(extension);
@@ -398,8 +419,9 @@ export class ExtensionDiscovery {
 
   /**
    * Loads extension from absolute path, updates this.packagesJson to include it and returns the extension.
+   * @param absPath Folder path to extension
    */
-  async loadExtensionFromPath(absPath: string, { isBundled = false }: {
+  async loadExtensionFromFolder(absPath: string, { isBundled = false }: {
     isBundled?: boolean;
   } = {}): Promise<InstalledExtension | null> {
     const manifestPath = path.resolve(absPath, manifestFilename);