fix: local sketch deletion + rename

Ref: #1825

Signed-off-by: Akos Kitta <[email protected]>

Author: Akos Kitta

arduino-ide-extension/src/browser/contributions/sketch-control.ts

@@ -18,33 +18,19 @@ import {
   open,
 } from './contribution';
 import { ArduinoMenus, PlaceholderMenuNode } from '../menu/arduino-menus';
-import { EditorManager } from '@theia/editor/lib/browser/editor-manager';
-import {
-  CurrentSketch,
-  SketchesServiceClientImpl,
-} from '../../common/protocol/sketches-service-client-impl';
-import { LocalCacheFsProvider } from '../local-cache/local-cache-fs-provider';
+import { CurrentSketch } from '../../common/protocol/sketches-service-client-impl';
 import { nls } from '@theia/core/lib/common';
 
 @injectable()
 export class SketchControl extends SketchContribution {
   @inject(ApplicationShell)
-  protected readonly shell: ApplicationShell;
+  private readonly shell: ApplicationShell;
 
   @inject(MenuModelRegistry)
-  protected readonly menuRegistry: MenuModelRegistry;
+  private readonly menuRegistry: MenuModelRegistry;
 
   @inject(ContextMenuRenderer)
-  protected readonly contextMenuRenderer: ContextMenuRenderer;
-
-  @inject(EditorManager)
-  protected override readonly editorManager: EditorManager;
-
-  @inject(SketchesServiceClientImpl)
-  protected readonly sketchesServiceClient: SketchesServiceClientImpl;
-
-  @inject(LocalCacheFsProvider)
-  protected readonly localCacheFsProvider: LocalCacheFsProvider;
+  private readonly contextMenuRenderer: ContextMenuRenderer;
 
   protected readonly toDisposeBeforeCreateNewContextMenu =
     new DisposableCollection();
@@ -84,12 +70,7 @@ export class SketchControl extends SketchContribution {
           );
 
           // if the current file is in the current opened sketch, show extra menus
-          if (
-            sketch &&
-            parentSketch &&
-            parentSketch.uri === sketch.uri &&
-            this.allowRename(parentSketch.uri)
-          ) {
+          if (sketch && parentSketch && parentSketch.uri === sketch.uri) {
             this.menuRegistry.registerMenuAction(
               ArduinoMenus.SKETCH_CONTROL__CONTEXT__MAIN_GROUP,
               {
@@ -121,12 +102,7 @@ export class SketchControl extends SketchContribution {
             );
           }
 
-          if (
-            sketch &&
-            parentSketch &&
-            parentSketch.uri === sketch.uri &&
-            this.allowDelete(parentSketch.uri)
-          ) {
+          if (sketch && parentSketch && parentSketch.uri === sketch.uri) {
             this.menuRegistry.registerMenuAction(
               ArduinoMenus.SKETCH_CONTROL__CONTEXT__MAIN_GROUP,
               {
@@ -249,27 +225,6 @@ export class SketchControl extends SketchContribution {
       command: SketchControl.Commands.OPEN_SKETCH_CONTROL__TOOLBAR.id,
     });
   }
-
-  protected isCloudSketch(uri: string): boolean {
-    try {
-      const cloudCacheLocation = this.localCacheFsProvider.from(new URI(uri));
-
-      if (cloudCacheLocation) {
-        return true;
-      }
-      return false;
-    } catch {
-      return false;
-    }
-  }
-
-  protected allowRename(uri: string): boolean {
-    return !this.isCloudSketch(uri);
-  }
-
-  protected allowDelete(uri: string): boolean {
-    return !this.isCloudSketch(uri);
-  }
 }
 
 export namespace SketchControl {

arduino-ide-extension/src/browser/create/create-api.ts

@@ -15,6 +15,8 @@ export namespace ResponseResultProvider {
   export const JSON: ResponseResultProvider = (response) => response.json();
 }
 
+// TODO: check if this is still needed: https://github.com/electron/electron/issues/18733
+// The original issue was reported for Electron 5.x and 6.x. Theia uses 15.x
 export function Utf8ArrayToStr(array: Uint8Array): string {
   let out, i, c;
   let char2, char3;
@@ -381,7 +383,7 @@ export class CreateApi {
       return;
     }
 
-    // do not upload "do_not_sync" files/directoris and their descendants
+    // do not upload "do_not_sync" files/directories and their descendants
     const segments = posixPath.split(posix.sep) || [];
     if (
       segments.some((segment) => Create.do_not_sync_files.includes(segment))

arduino-ide-extension/src/browser/theia/workspace/workspace-delete-handler.ts

@@ -1,17 +1,28 @@
-import { inject, injectable } from '@theia/core/shared/inversify';
+import type { Widget } from '@phosphor/widgets';
 import * as remote from '@theia/core/electron-shared/@electron/remote';
+import { Dialog } from '@theia/core/lib/browser/dialogs';
+import { NavigatableWidget } from '@theia/core/lib/browser/navigatable-types';
+import { WindowService } from '@theia/core/lib/browser/window/window-service';
+import { nls } from '@theia/core/lib/common';
+import type { MaybeArray } from '@theia/core/lib/common/types';
 import URI from '@theia/core/lib/common/uri';
+import { inject, injectable } from '@theia/core/shared/inversify';
 import { WorkspaceDeleteHandler as TheiaWorkspaceDeleteHandler } from '@theia/workspace/lib/browser/workspace-delete-handler';
+import { SketchesService } from '../../../common/protocol';
 import {
   CurrentSketch,
   SketchesServiceClientImpl,
 } from '../../../common/protocol/sketches-service-client-impl';
-import { nls } from '@theia/core/lib/common';
+import { Sketch } from '../../contributions/contribution';
 
 @injectable()
 export class WorkspaceDeleteHandler extends TheiaWorkspaceDeleteHandler {
+  @inject(WindowService)
+  private readonly windowService: WindowService;
+  @inject(SketchesService)
+  private readonly sketchesService: SketchesService;
   @inject(SketchesServiceClientImpl)
-  protected readonly sketchesServiceClient: SketchesServiceClientImpl;
+  private readonly sketchesServiceClient: SketchesServiceClientImpl;
 
   override async execute(uris: URI[]): Promise<void> {
     const sketch = await this.sketchesServiceClient.currentSketch();
@@ -27,29 +38,48 @@ export class WorkspaceDeleteHandler extends TheiaWorkspaceDeleteHandler {
       const { response } = await remote.dialog.showMessageBox({
         title: nls.localize('vscode/fileActions/delete', 'Delete'),
         type: 'question',
-        buttons: [
-          nls.localize('vscode/issueMainService/cancel', 'Cancel'),
-          nls.localize('vscode/issueMainService/ok', 'OK'),
-        ],
+        buttons: [Dialog.CANCEL, Dialog.OK],
         message: nls.localize(
           'theia/workspace/deleteCurrentSketch',
           'Do you want to delete the current sketch?'
         ),
       });
       if (response === 1) {
         // OK
-        await Promise.all(
-          [
-            ...sketch.additionalFileUris,
-            ...sketch.otherSketchFileUris,
-            sketch.mainFileUri,
-          ].map((uri) => this.closeWithoutSaving(new URI(uri)))
-        );
-        await this.fileService.delete(new URI(sketch.uri));
-        window.close();
+        await Promise.all([
+          ...Sketch.uris(sketch).map((uri) =>
+            this.closeWithoutSaving(new URI(uri))
+          ),
+        ]);
+        this.windowService.setSafeToShutDown();
+        this.sketchesService.deleteSketch(sketch);
+        return window.close();
       }
-      return;
     }
     return super.execute(uris);
   }
+
+  // https://github.com/eclipse-theia/theia/issues/12107
+  protected override async closeWithoutSaving(uri: URI): Promise<void> {
+    const affected = getAffected(this.shell.widgets, uri);
+    const toClose = [...affected].map(([, widget]) => widget);
+    await this.shell.closeMany(toClose, { save: false });
+  }
+}
+
+export function getAffected<T extends Widget>(
+  widgets: Iterable<T>,
+  context: MaybeArray<URI>
+): [URI, T & NavigatableWidget][] {
+  const uris = Array.isArray(context) ? context : [context];
+  const result: [URI, T & NavigatableWidget][] = [];
+  for (const widget of widgets) {
+    if (NavigatableWidget.is(widget)) {
+      const resourceUri = widget.getResourceUri();
+      if (resourceUri && uris.some((uri) => uri.isEqualOrParent(resourceUri))) {
+        result.push([resourceUri, widget]);
+      }
+    }
+  }
+  return result;
 }

arduino-ide-extension/src/browser/widgets/cloud-sketchbook/cloud-sketchbook-tree.ts

@@ -2,7 +2,6 @@ import { SketchCache } from './cloud-sketch-cache';
 import { inject, injectable } from '@theia/core/shared/inversify';
 import URI from '@theia/core/lib/common/uri';
 import { MaybePromise } from '@theia/core/lib/common/types';
-import { FileService } from '@theia/filesystem/lib/browser/file-service';
 import { FileStatNode } from '@theia/filesystem/lib/browser/file-tree';
 import { Command } from '@theia/core/lib/common/command';
 import { WidgetDecoration } from '@theia/core/lib/browser/widget-decoration';
@@ -28,8 +27,6 @@ import { CloudSketchbookCommands } from './cloud-sketchbook-contributions';
 import { DoNotAskAgainConfirmDialog } from '../../dialogs/do-not-ask-again-dialog';
 import { SketchbookTree } from '../sketchbook/sketchbook-tree';
 import { firstToUpperCase } from '../../../common/utils';
-import { ArduinoPreferences } from '../../arduino-preferences';
-import { SketchesServiceClientImpl } from '../../../common/protocol/sketches-service-client-impl';
 import { FileStat } from '@theia/filesystem/lib/common/files';
 import { WorkspaceNode } from '@theia/navigator/lib/browser/navigator-tree';
 import { posix, splitSketchPath } from '../../create/create-paths';
@@ -46,29 +43,20 @@ type FilesToSync = {
 };
 @injectable()
 export class CloudSketchbookTree extends SketchbookTree {
-  @inject(FileService)
-  protected override readonly fileService: FileService;
-
   @inject(LocalCacheFsProvider)
-  protected readonly localCacheFsProvider: LocalCacheFsProvider;
+  private readonly localCacheFsProvider: LocalCacheFsProvider;
 
   @inject(SketchCache)
-  protected readonly sketchCache: SketchCache;
-
-  @inject(ArduinoPreferences)
-  protected override readonly arduinoPreferences: ArduinoPreferences;
+  private readonly sketchCache: SketchCache;
 
   @inject(PreferenceService)
-  protected readonly preferenceService: PreferenceService;
+  private readonly preferenceService: PreferenceService;
 
   @inject(MessageService)
-  protected readonly messageService: MessageService;
-
-  @inject(SketchesServiceClientImpl)
-  protected readonly sketchServiceClient: SketchesServiceClientImpl;
+  private readonly messageService: MessageService;
 
   @inject(CreateApi)
-  protected readonly createApi: CreateApi;
+  private readonly createApi: CreateApi;
 
   async pushPublicWarn(
     node: CloudSketchbookTree.CloudSketchDirNode
@@ -93,15 +81,13 @@ export class CloudSketchbookTree extends SketchbookTree {
             PreferenceScope.User
           ),
       }).open();
-      if (!ok) {
-        return false;
-      }
-      return true;
+      return Boolean(ok);
     } else {
       return true;
     }
   }
 
+  // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types, @typescript-eslint/no-explicit-any
   async pull(arg: any): Promise<void> {
     const {
       // model,
@@ -136,6 +122,7 @@ export class CloudSketchbookTree extends SketchbookTree {
         return;
       }
     }
+    // this.sketchCache.init();
     return this.runWithState(node, 'pulling', async (node) => {
       const commandsCopy = node.commands;
       node.commands = [];
@@ -229,7 +216,7 @@ export class CloudSketchbookTree extends SketchbookTree {
     });
   }
 
-  async recursiveURIs(uri: URI): Promise<URI[]> {
+  private async recursiveURIs(uri: URI): Promise<URI[]> {
     // remote resources can be fetched one-shot via api
     if (CreateUri.is(uri)) {
       const resources = await this.createApi.readDirectory(
@@ -286,15 +273,15 @@ export class CloudSketchbookTree extends SketchbookTree {
     }, {});
   }
 
-  async getUrisMap(uri: URI) {
+  private async getUrisMap(uri: URI): Promise<Record<string, URI>> {
     const basepath = uri.toString();
     const exists = await this.fileService.exists(uri);
     const uris =
       (exists && this.URIsToMap(await this.recursiveURIs(uri), basepath)) || {};
     return uris;
   }
 
-  async treeDiff(source: URI, dest: URI): Promise<FilesToSync> {
+  private async treeDiff(source: URI, dest: URI): Promise<FilesToSync> {
     const [sourceURIs, destURIs] = await Promise.all([
       this.getUrisMap(source),
       this.getUrisMap(dest),
@@ -356,7 +343,7 @@ export class CloudSketchbookTree extends SketchbookTree {
     }
   }
 
-  async sync(source: URI, dest: URI) {
+  private async sync(source: URI, dest: URI): Promise<void> {
     const { filesToWrite, filesToDelete } = await this.treeDiff(source, dest);
     await Promise.all(
       filesToWrite.map(async ({ source, dest }) => {
@@ -375,7 +362,9 @@ export class CloudSketchbookTree extends SketchbookTree {
     );
   }
 
-  override async resolveChildren(parent: CompositeTreeNode): Promise<TreeNode[]> {
+  override async resolveChildren(
+    parent: CompositeTreeNode
+  ): Promise<TreeNode[]> {
     return (await super.resolveChildren(parent)).sort((a, b) => {
       if (
         WorkspaceNode.is(parent) &&
@@ -453,6 +442,7 @@ export class CloudSketchbookTree extends SketchbookTree {
           if (!CreateUri.is(childFs.resource)) {
             let refUri = node.fileStat.resource;
             if (node.fileStat.hasOwnProperty('remoteUri')) {
+              // eslint-disable-next-line @typescript-eslint/no-explicit-any
               refUri = (node.fileStat as any).remoteUri;
             }
             remoteUri = refUri.resolve(childFs.name);
@@ -471,6 +461,7 @@ export class CloudSketchbookTree extends SketchbookTree {
   }
 
   protected override toNode(
+    // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types, @typescript-eslint/no-explicit-any
     fileStat: any,
     parent: CompositeTreeNode
   ): FileNode | DirNode {
@@ -621,6 +612,7 @@ export class CloudSketchbookTree extends SketchbookTree {
     if (DecoratedTreeNode.is(node)) {
       for (const property of Object.keys(decorationData)) {
         if (node.decorationData.hasOwnProperty(property)) {
+          // eslint-disable-next-line @typescript-eslint/no-explicit-any
           delete (node.decorationData as any)[property];
         }
       }

arduino-ide-extension/src/node/arduino-ide-backend-module.ts

@@ -196,6 +196,7 @@ export default new ContainerModule((bind, unbind, isBound, rebind) => {
   // Shared sketches service
   bind(SketchesServiceImpl).toSelf().inSingletonScope();
   bind(SketchesService).toService(SketchesServiceImpl);
+  bind(BackendApplicationContribution).toService(SketchesServiceImpl);
   bind(ConnectionHandler)
     .toDynamicValue(
       (context) =>

arduino-ide-extension/src/node/boards-service-impl.ts

@@ -7,7 +7,6 @@ import {
   BoardsPackage,
   Board,
   BoardDetails,
-  Tool,
   ConfigOption,
   ConfigValue,
   Programmer,
@@ -97,14 +96,11 @@ export class BoardsServiceImpl
 
     const debuggingSupported = detailsResp.getDebuggingSupported();
 
-    const requiredTools = detailsResp.getToolsDependenciesList().map(
-      (t) =>
-        <Tool>{
-          name: t.getName(),
-          packager: t.getPackager(),
-          version: t.getVersion(),
-        }
-    );
+    const requiredTools = detailsResp.getToolsDependenciesList().map((t) => ({
+      name: t.getName(),
+      packager: t.getPackager(),
+      version: t.getVersion(),
+    }));
 
     const configOptions = detailsResp.getConfigOptionsList().map(
       (c) =>

arduino-ide-extension/src/node/sketches-service-impl.ts

@@ -1,5 +1,13 @@
 import { injectable, inject, named } from '@theia/core/shared/inversify';
-import { promises as fs, realpath, lstat, Stats, constants, rm } from 'fs';
+import {
+  promises as fs,
+  realpath,
+  lstat,
+  Stats,
+  constants,
+  rm,
+  rmSync,
+} from 'fs';
 import * as os from 'os';
 import * as temp from 'temp';
 import * as path from 'path';
@@ -43,6 +51,11 @@ import {
   firstToUpperCase,
   startsWithUpperCase,
 } from '../common/utils';
+import { BackendApplicationContribution } from '@theia/core/lib/node/backend-application';
+import {
+  Disposable,
+  DisposableCollection,
+} from '@theia/core/lib/common/disposable';
 
 const RecentSketches = 'recent-sketches.json';
 const DefaultIno = `void setup() {
@@ -59,7 +72,7 @@ void loop() {
 @injectable()
 export class SketchesServiceImpl
   extends CoreClientAware
-  implements SketchesService
+  implements SketchesService, BackendApplicationContribution
 {
   private sketchSuffixIndex = 1;
   private lastSketchBaseName: string;
@@ -69,6 +82,32 @@ export class SketchesServiceImpl
     concurrency: 1,
   });
   private inoContent: Deferred<string> | undefined;
+  /**
+   * It contains all things the IDE2 must clean up before a normal stop.
+   *
+   * When deleting the sketch, the IDE2 must close the browser window and
+   * recursively delete the sketch folder from the filesystem. The sketch
+   * cannot be deleted when the window is open because that is the currently
+   * opened workspace. IDE2 cannot delete the sketch folder from the
+   * filesystem after closing the browser window because the window can be
+   * the last, and when the last window closes, the application quits.
+   * There is no way to clean up the undesired resources.
+   *
+   * This array contains disposable instances wrapping synchronous sketch
+   * delete operations. When IDE2 closes the browser window, it schedules
+   * the sketch deletion, and the window closes.
+   *
+   * When IDE2 schedules a sketch for deletion, it creates a synchronous
+   * folder deletion as a disposable instance and pushes it into this
+   * array. After the push, IDE2 starts the sketch deletion in an
+   * asynchronous way. When the deletion completes, the disposable is
+   * removed. If the app quits when the asynchronous deletion is still in
+   * progress, it disposes the elements of this array. Since it is
+   * synchronous, it is [ensured by Theia](https://github.com/eclipse-theia/theia/blob/678e335644f1b38cb27522cc27a3b8209293cf31/packages/core/src/node/backend-application.ts#L91-L97)
+   * that IDE2 won't quit before the cleanup is done. It works only in normal
+   * quit.
+   */
+  private readonly scheduledDeletions: Disposable[] = [];
 
   @inject(ILogger)
   @named('sketches-service')
@@ -86,6 +125,14 @@ export class SketchesServiceImpl
   @inject(IsTempSketch)
   private readonly isTempSketch: IsTempSketch;
 
+  onStop(): void {
+    if (this.scheduledDeletions.length) {
+      this.logger.info(`>>> Disposing sketches service...`);
+      new DisposableCollection(...this.scheduledDeletions).dispose();
+      this.logger.info(`<<< Disposed sketches service.`);
+    }
+  }
+
   async getSketches({ uri }: { uri?: string }): Promise<SketchContainer> {
     const root = await this.root(uri);
     if (!root) {
@@ -629,20 +676,47 @@ export class SketchesServiceImpl
   }
 
   async deleteSketch(sketch: Sketch): Promise<void> {
+    const sketchPath = FileUri.fsPath(sketch.uri);
+    const disposable = Disposable.create(() =>
+      this.deleteSketchSync(sketchPath)
+    );
+    this.scheduledDeletions.push(disposable);
     return new Promise<void>((resolve, reject) => {
-      const sketchPath = FileUri.fsPath(sketch.uri);
       rm(sketchPath, { recursive: true, maxRetries: 5 }, (error) => {
         if (error) {
           this.logger.error(`Failed to delete sketch at ${sketchPath}.`, error);
           reject(error);
         } else {
           this.logger.info(`Successfully deleted sketch at ${sketchPath}.`);
           resolve();
+          const index = this.scheduledDeletions.indexOf(disposable);
+          if (index >= 0) {
+            this.scheduledDeletions.splice(index, 1);
+          } else {
+            this.logger.warn(
+              `Could not find the scheduled sketch deletion: ${sketchPath}`
+            );
+          }
         }
       });
     });
   }
 
+  private deleteSketchSync(sketchPath: string): void {
+    this.logger.info(
+      `>>> Running sketch deletion ${sketchPath} before app quit...`
+    );
+    try {
+      rmSync(sketchPath, { recursive: true, maxRetries: 5 });
+      this.logger.info(`<<< Deleted sketch ${sketchPath}.`);
+    } catch (err) {
+      if (ErrnoException.isENOENT(err)) {
+        // ignore. it does not exist
+      }
+      throw err;
+    }
+  }
+
   // Returns the default.ino from the settings or from default folder.
   private async readSettings(): Promise<Record<string, unknown> | undefined> {
     const configDirUri = await this.envVariableServer.getConfigDirUri();