Fix file system delete of folder

Deleting a folder without recursive enabled failed as the used nodejs API unlink does not support folder deletion. Fixing it by first checking if it is a folder, and if so using rmdir instead. Added a simple unit test, which also exposed that the folder deletion also fails when using trash support, but that is a different issue. Signed-off-by: Daniel Friederich <[email protected]>
kittaakos · Feb 9, 2024 · 3034c0a · 3034c0a
1 parent b7ed5e9
commit 3034c0a
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 2 deletions.
diff --git a/packages/filesystem/src/node/disk-file-system-provider.spec.ts b/packages/filesystem/src/node/disk-file-system-provider.spec.ts
@@ -26,7 +26,7 @@ import { promises as fs } from 'fs';
 import { join } from 'path';
 import * as temp from 'temp';
 import { v4 } from 'uuid';
-import { FilePermission, FileSystemProviderError, FileSystemProviderErrorCode } from '../common/files';
+import { FilePermission, FileSystemProviderCapabilities, FileSystemProviderError, FileSystemProviderErrorCode } from '../common/files';
 import { DiskFileSystemProvider } from './disk-file-system-provider';
 import { bindFileSystemWatcherServer } from './filesystem-backend-module';
 
@@ -93,6 +93,39 @@ describe('disk-file-system-provider', () => {
         });
     });
 
+    describe('delete', () => {
+        it('delete is able to delete folder', async () => {
+            const tempDirPath = tracked.mkdirSync();
+            const testFolder = join(tempDirPath, 'test');
+            const folderUri = FileUri.create(testFolder);
+            for (const recursive of [true, false]) {
+                // Note: useTrash = true fails on Linux
+                const useTrash = false;
+                if ((fsProvider.capabilities & FileSystemProviderCapabilities.Access) === 0 && useTrash) {
+                    continue;
+                }
+                await fsProvider.mkdir(folderUri);
+                if (recursive) {
+                    await fsProvider.writeFile(FileUri.create(join(testFolder, 'test.file')), Buffer.from('test'), { overwrite: false, create: true });
+                    await fsProvider.mkdir(FileUri.create(join(testFolder, 'subFolder')));
+                }
+                await fsProvider.delete(folderUri, { recursive, useTrash });
+            }
+        });
+
+        it('delete is able to delete file', async () => {
+            const tempDirPath = tracked.mkdirSync();
+            const testFile = join(tempDirPath, 'test.file');
+            const testFileUri = FileUri.create(testFile);
+            for (const recursive of [true, false]) {
+                for (const useTrash of [true, false]) {
+                    await fsProvider.writeFile(testFileUri, Buffer.from('test'), { overwrite: false, create: true });
+                    await fsProvider.delete(testFileUri, { recursive, useTrash });
+                }
+            }
+        });
+    });
+
     function createContainer(): Container {
         const container = new Container({ defaultScope: 'Singleton' });
         const module = new ContainerModule(bind => {

diff --git a/packages/filesystem/src/node/disk-file-system-provider.ts b/packages/filesystem/src/node/disk-file-system-provider.ts
@@ -509,7 +509,12 @@ export class DiskFileSystemProvider implements Disposable,
             if (opts.recursive) {
                 await this.rimraf(filePath);
             } else {
-                await promisify(unlink)(filePath);
+                const stat = await promisify(lstat)(filePath);
+                if (stat.isDirectory() && !stat.isSymbolicLink()) {
+                    await promisify(rmdir)(filePath);
+                } else {
+                    await promisify(unlink)(filePath);
+                }
             }
         } else {
             await trash(filePath);