From ec02f555f055e05f4ba9b6e486a2a538d221e023 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 21 Oct 2019 15:46:20 -0600 Subject: [PATCH 01/25] Use the new API for deleting files and directories. --- src/client/common/platform/fileSystem.ts | 47 ++++++-- .../platform/filesystem.functional.test.ts | 84 ++----------- src/test/common/platform/filesystem.test.ts | 113 ++++++++++++++++++ .../common/platform/filesystem.unit.test.ts | 24 ++-- 4 files changed, 169 insertions(+), 99 deletions(-) create mode 100644 src/test/common/platform/filesystem.test.ts diff --git a/src/client/common/platform/fileSystem.ts b/src/client/common/platform/fileSystem.ts index 25886aabc12d..a3b5769c1a40 100644 --- a/src/client/common/platform/fileSystem.ts +++ b/src/client/common/platform/fileSystem.ts @@ -112,6 +112,18 @@ export class TempFileSystem { } } +// This is the parts of the vscode.workspace.fs API that we use here. +interface INewAPI { + //copy(source: vscode.Uri, target: vscode.Uri, options?: {overwrite: boolean}): Thenable; + //createDirectory(uri: vscode.Uri): Thenable; + delete(uri: vscode.Uri, options?: {recursive: boolean; useTrash: boolean}): Thenable; + //readDirectory(uri: vscode.Uri): Thenable<[string, FileType][]>; + //readFile(uri: vscode.Uri): Thenable; + //rename(source: vscode.Uri, target: vscode.Uri, options?: {overwrite: boolean}): Thenable; + //stat(uri: vscode.Uri): Thenable; + //writeFile(uri: vscode.Uri, content: Uint8Array): Thenable; +} + // This is the parts of node's 'fs' module that we use in RawFileSystem. interface IRawFS { // non-async @@ -124,13 +136,10 @@ interface IRawFSExtra { readFile(path: string, encoding: string): Promise; //tslint:disable-next-line:no-any writeFile(path: string, data: any, options: any): Promise; - unlink(filename: string): Promise; stat(filename: string): Promise; lstat(filename: string): Promise; mkdirp(dirname: string): Promise; - rmdir(dirname: string): Promise; readdir(dirname: string): Promise; - remove(dirname: string): Promise; // non-async statSync(filename: string): fsextra.Stats; @@ -151,6 +160,7 @@ interface IRawPath { export class RawFileSystem implements IRawFileSystem { constructor( protected readonly path: IRawPath, + protected readonly newapi: INewAPI, protected readonly nodefs: IRawFS, protected readonly fsExtra: IRawFSExtra ) { } @@ -159,11 +169,31 @@ export class RawFileSystem implements IRawFileSystem { public static withDefaults(): RawFileSystem{ return new RawFileSystem( FileSystemPaths.withDefaults(), + vscode.workspace.fs, fs, fsextra ); } + //**************************** + // VS Code API + + public async rmtree(dirname: string): Promise { + const uri = vscode.Uri.file(dirname); + return this.newapi.delete(uri, { + recursive: true, + useTrash: false + }); + } + + public async rmfile(filename: string): Promise { + const uri = vscode.Uri.file(filename); + return this.newapi.delete(uri, { + recursive: false, + useTrash: false + }); + } + //**************************** // fs-extra @@ -182,15 +212,6 @@ export class RawFileSystem implements IRawFileSystem { return this.fsExtra.mkdirp(dirname); } - public async rmtree(dirname: string): Promise { - return this.fsExtra.stat(dirname) - .then(() => this.fsExtra.remove(dirname)); - } - - public async rmfile(filename: string): Promise { - return this.fsExtra.unlink(filename); - } - public async chmod(filename: string, mode: string | number): Promise { return this.fsExtra.chmod(filename, mode); } @@ -266,7 +287,7 @@ export class FileSystemUtils implements IFileSystemUtils { public static withDefaults(): FileSystemUtils { const paths = FileSystemPaths.withDefaults(); return new FileSystemUtils( - new RawFileSystem(paths, fs, fsextra), + new RawFileSystem(paths, vscode.workspace.fs, fs, fsextra), paths, TempFileSystem.withDefaults(), getHashString, diff --git a/src/test/common/platform/filesystem.functional.test.ts b/src/test/common/platform/filesystem.functional.test.ts index 24419c7369d8..7451271dbcc4 100644 --- a/src/test/common/platform/filesystem.functional.test.ts +++ b/src/test/common/platform/filesystem.functional.test.ts @@ -24,23 +24,26 @@ const assertArrays = require('chai-arrays'); use(assertArrays); use(chaiAsPromised); -const WINDOWS = /^win/.test(process.platform); +// Note: all functional tests that trigger the VS Code "fs" API are +// found in filesystem.test.ts. -const DOES_NOT_EXIST = 'this file does not exist'; +export const WINDOWS = /^win/.test(process.platform); -async function assertDoesNotExist(filename: string) { +export const DOES_NOT_EXIST = 'this file does not exist'; + +export async function assertDoesNotExist(filename: string) { await expect( fsextra.stat(filename) ).to.eventually.be.rejected; } -async function assertExists(filename: string) { +export async function assertExists(filename: string) { await expect( fsextra.stat(filename) ).to.not.eventually.be.rejected; } -class FSFixture { +export class FSFixture { public tempDir: tmpMod.SynchrounousResult | undefined; public sockServer: net.Server | undefined; @@ -293,41 +296,6 @@ suite('Raw FileSystem', () => { }); }); - suite('rmtree', () => { - test('deletes the directory and everything in it', async () => { - const dirname = await fix.createDirectory('x'); - const filename = await fix.createFile('x/y/z/spam.py'); - await assertExists(filename); - - await filesystem.rmtree(dirname); - - await assertDoesNotExist(dirname); - }); - - test('fails if the directory does not exist', async () => { - const promise = filesystem.rmtree(DOES_NOT_EXIST); - - await expect(promise).to.eventually.be.rejected; - }); - }); - - suite('rmfile', () => { - test('deletes the file', async () => { - const filename = await fix.createFile('x/y/z/spam.py', '...'); - await assertExists(filename); - - await filesystem.rmfile(filename); - - await assertDoesNotExist(filename); - }); - - test('fails if the file does not exist', async () => { - const promise = filesystem.rmfile(DOES_NOT_EXIST); - - await expect(promise).to.eventually.be.rejected; - }); - }); - suite('chmod (non-Windows)', () => { suiteSetup(function () { // On Windows, chmod won't have any effect on the file itself. @@ -336,7 +304,6 @@ suite('Raw FileSystem', () => { this.skip(); } }); - async function checkMode(filename: string, expected: number) { const stat = await fsextra.stat(filename); expect(stat.mode & 0o777).to.equal(expected); @@ -871,41 +838,6 @@ suite('FileSystem Utils', () => { }); }); - suite('isDirReadonly', () => { - suite('non-Windows', () => { - suiteSetup(function () { - if (WINDOWS) { - // tslint:disable-next-line:no-invalid-this - this.skip(); - } - }); - - // On Windows, chmod won't have any effect on the file itself. - test('is readonly', async () => { - const dirname = await fix.createDirectory('x/y/z/spam'); - await fsextra.chmod(dirname, 0o444); - - const isReadonly = await utils.isDirReadonly(dirname); - - expect(isReadonly).to.equal(true); - }); - }); - - test('is not readonly', async () => { - const dirname = await fix.createDirectory('x/y/z/spam'); - - const isReadonly = await utils.isDirReadonly(dirname); - - expect(isReadonly).to.equal(false); - }); - - test('fails if the file does not exist', async () => { - const promise = utils.isDirReadonly(DOES_NOT_EXIST); - - await expect(promise).to.eventually.be.rejected; - }); - }); - suite('getFileHash', () => { test('Getting hash for a file should return non-empty string', async () => { const filename = await fix.createFile('x/y/z/spam.py'); diff --git a/src/test/common/platform/filesystem.test.ts b/src/test/common/platform/filesystem.test.ts new file mode 100644 index 000000000000..b7822d51c296 --- /dev/null +++ b/src/test/common/platform/filesystem.test.ts @@ -0,0 +1,113 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import { expect } from 'chai'; +import * as fsextra from 'fs-extra'; +import { + FileSystemUtils, RawFileSystem +} from '../../../client/common/platform/fileSystem'; +import { + IFileSystemUtils, IRawFileSystem +} from '../../../client/common/platform/types'; +import { + DOES_NOT_EXIST, assertDoesNotExist, assertExists, FSFixture, WINDOWS +} from './filesystem.functional.test'; + +// Note: all functional tests that do not trigger the VS Code "fs" API +// are found in filesystem.functional.test.ts. + +// tslint:disable:max-func-body-length chai-vague-errors + +suite('Raw FileSystem', () => { + let filesystem: IRawFileSystem; + let fix: FSFixture; + setup(() => { + filesystem = RawFileSystem.withDefaults(); + fix = new FSFixture(); + }); + teardown(async () => { + await fix.cleanUp(); + }); + + suite('rmtree', () => { + test('deletes the directory and everything in it', async () => { + const dirname = await fix.createDirectory('x'); + const filename = await fix.createFile('x/y/z/spam.py'); + await assertExists(filename); + + await filesystem.rmtree(dirname); + + await assertDoesNotExist(dirname); + }); + + test('fails if the directory does not exist', async () => { + const promise = filesystem.rmtree(DOES_NOT_EXIST); + + await expect(promise).to.eventually.be.rejected; + }); + }); + + suite('rmfile', () => { + test('deletes the file', async () => { + const filename = await fix.createFile('x/y/z/spam.py', '...'); + await assertExists(filename); + + await filesystem.rmfile(filename); + + await assertDoesNotExist(filename); + }); + + test('fails if the file does not exist', async () => { + const promise = filesystem.rmfile(DOES_NOT_EXIST); + + await expect(promise).to.eventually.be.rejected; + }); + }); +}); + +suite('FileSystem Utils', () => { + let utils: IFileSystemUtils; + let fix: FSFixture; + setup(() => { + utils = FileSystemUtils.withDefaults(); + fix = new FSFixture(); + }); + teardown(async () => { + await fix.cleanUp(); + }); + + suite('isDirReadonly', () => { + suite('non-Windows', () => { + suiteSetup(function () { + if (WINDOWS) { + // tslint:disable-next-line:no-invalid-this + this.skip(); + } + }); + + // On Windows, chmod won't have any effect on the file itself. + test('is readonly', async () => { + const dirname = await fix.createDirectory('x/y/z/spam'); + await fsextra.chmod(dirname, 0o444); + + const isReadonly = await utils.isDirReadonly(dirname); + + expect(isReadonly).to.equal(true); + }); + }); + + test('is not readonly', async () => { + const dirname = await fix.createDirectory('x/y/z/spam'); + + const isReadonly = await utils.isDirReadonly(dirname); + + expect(isReadonly).to.equal(false); + }); + + test('fails if the file does not exist', async () => { + const promise = utils.isDirReadonly(DOES_NOT_EXIST); + + await expect(promise).to.eventually.be.rejected; + }); + }); +}); diff --git a/src/test/common/platform/filesystem.unit.test.ts b/src/test/common/platform/filesystem.unit.test.ts index ac5d7c096109..a7f5a8a225d6 100644 --- a/src/test/common/platform/filesystem.unit.test.ts +++ b/src/test/common/platform/filesystem.unit.test.ts @@ -4,7 +4,7 @@ import { expect } from 'chai'; import * as fsextra from 'fs-extra'; import * as TypeMoq from 'typemoq'; -import { Disposable } from 'vscode'; +import { Disposable, Uri } from 'vscode'; import { FileSystemPaths, FileSystemUtils, RawFileSystem, TempFileSystem } from '../../../client/common/platform/fileSystem'; @@ -19,18 +19,24 @@ import { //tslint:disable-next-line:no-any type TempCallback = (err: any, path: string, fd: number, cleanupCallback: () => void) => void; interface IRawFS { + // VS Code + delete(uri: Uri, options?: {recursive: boolean; useTrash: boolean}): Thenable; + + // node "fs" + //tslint:disable-next-line:no-any + open(filename: string, flags: number, callback: any): void; + //tslint:disable-next-line:no-any + close(fd: number, callback: any): void; + // "fs-extra" chmod(filePath: string, mode: string): Promise; readFile(path: string, encoding: string): Promise; //tslint:disable-next-line:no-any writeFile(path: string, data: any, options: any): Promise; - unlink(filename: string): Promise; stat(filename: string): Promise; lstat(filename: string): Promise; mkdirp(dirname: string): Promise; - rmdir(dirname: string): Promise; readdir(dirname: string): Promise; - remove(dirname: string): Promise; statSync(filename: string): fsextra.Stats; readFileSync(path: string, encoding: string): string; createReadStream(src: string): fsextra.ReadStream; @@ -235,6 +241,7 @@ suite('Raw FileSystem', () => { setup(() => { raw = TypeMoq.Mock.ofType(undefined, TypeMoq.MockBehavior.Strict); filesystem = new RawFileSystem( + raw.object, raw.object, raw.object, raw.object @@ -298,10 +305,7 @@ suite('Raw FileSystem', () => { suite('rmtree', () => { test('wraps the low-level function', async () => { const dirname = 'x/y/z/spam'; - raw.setup(r => r.stat(dirname)) - //tslint:disable-next-line:no-any - .returns(() => Promise.resolve({} as any as FileStat)); - raw.setup(r => r.remove(dirname)) + raw.setup(r => r.delete(Uri.file(dirname), { recursive: true, useTrash: false })) .returns(() => Promise.resolve()); await filesystem.rmtree(dirname); @@ -311,7 +315,7 @@ suite('Raw FileSystem', () => { test('fails if the directory does not exist', async () => { const dirname = 'x/y/z/spam'; - raw.setup(r => r.stat(dirname)) + raw.setup(r => r.delete(Uri.file(dirname), { recursive: true, useTrash: false })) .throws(new Error('file not found')); const promise = filesystem.rmtree(dirname); @@ -323,7 +327,7 @@ suite('Raw FileSystem', () => { suite('rmfile', () => { test('wraps the low-level function', async () => { const filename = 'x/y/z/spam.py'; - raw.setup(r => r.unlink(filename)) + raw.setup(r => r.delete(Uri.file(filename), { recursive: false, useTrash: false })) .returns(() => Promise.resolve()); await filesystem.rmfile(filename); From 80fd402f2bad2b2b3b64e07afd2b612f54fa321e Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Tue, 22 Oct 2019 16:01:25 -0600 Subject: [PATCH 02/25] Move listdir() to the new API. --- src/client/common/platform/fileSystem.ts | 47 ++------ .../platform/filesystem.functional.test.ts | 113 ----------------- src/test/common/platform/filesystem.test.ts | 114 ++++++++++++++++++ .../common/platform/filesystem.unit.test.ts | 64 ++-------- 4 files changed, 131 insertions(+), 207 deletions(-) diff --git a/src/client/common/platform/fileSystem.ts b/src/client/common/platform/fileSystem.ts index a3b5769c1a40..0c8f093ab21e 100644 --- a/src/client/common/platform/fileSystem.ts +++ b/src/client/common/platform/fileSystem.ts @@ -25,19 +25,6 @@ import { const ENCODING: string = 'utf8'; -// Determine the file type from the given file info. -function getFileType(stat: FileStat): FileType { - if (stat.isFile()) { - return FileType.File; - } else if (stat.isDirectory()) { - return FileType.Directory; - } else if (stat.isSymbolicLink()) { - return FileType.SymbolicLink; - } else { - return FileType.Unknown; - } -} - // The parts of node's 'path' module used by FileSystemPath. interface INodePath { join(...filenames: string[]): string; @@ -117,7 +104,7 @@ interface INewAPI { //copy(source: vscode.Uri, target: vscode.Uri, options?: {overwrite: boolean}): Thenable; //createDirectory(uri: vscode.Uri): Thenable; delete(uri: vscode.Uri, options?: {recursive: boolean; useTrash: boolean}): Thenable; - //readDirectory(uri: vscode.Uri): Thenable<[string, FileType][]>; + readDirectory(uri: vscode.Uri): Thenable<[string, FileType][]>; //readFile(uri: vscode.Uri): Thenable; //rename(source: vscode.Uri, target: vscode.Uri, options?: {overwrite: boolean}): Thenable; //stat(uri: vscode.Uri): Thenable; @@ -139,7 +126,6 @@ interface IRawFSExtra { stat(filename: string): Promise; lstat(filename: string): Promise; mkdirp(dirname: string): Promise; - readdir(dirname: string): Promise; // non-async statSync(filename: string): fsextra.Stats; @@ -148,18 +134,12 @@ interface IRawFSExtra { createWriteStream(dest: string): fsextra.WriteStream; } -// The parts of IFileSystemPaths used by RawFileSystem. -interface IRawPath { - join(...filenames: string[]): string; -} - // Later we will drop "FileSystem", switching usage to // "FileSystemUtils" and then rename "RawFileSystem" to "FileSystem". // The low-level filesystem operations used by the extension. export class RawFileSystem implements IRawFileSystem { constructor( - protected readonly path: IRawPath, protected readonly newapi: INewAPI, protected readonly nodefs: IRawFS, protected readonly fsExtra: IRawFSExtra @@ -168,7 +148,6 @@ export class RawFileSystem implements IRawFileSystem { // Create a new object using common-case default values. public static withDefaults(): RawFileSystem{ return new RawFileSystem( - FileSystemPaths.withDefaults(), vscode.workspace.fs, fs, fsextra @@ -194,6 +173,11 @@ export class RawFileSystem implements IRawFileSystem { }); } + public async listdir(dirname: string): Promise<[string, FileType][]> { + const uri = vscode.Uri.file(dirname); + return this.newapi.readDirectory(uri); + } + //**************************** // fs-extra @@ -224,20 +208,6 @@ export class RawFileSystem implements IRawFileSystem { return this.fsExtra.lstat(filename); } - // Once we move to the VS Code API, this method becomes a trivial wrapper. - public async listdir(dirname: string): Promise<[string, FileType][]> { - const names: string[] = await this.fsExtra.readdir(dirname); - const promises = names - .map(name => { - const filename = this.path.join(dirname, name); - return this.lstat(filename) - .then(stat => [name, getFileType(stat)] as [string, FileType]) - .catch(() => [name, FileType.Unknown] as [string, FileType]); - }); - return Promise.all(promises); - } - - // Once we move to the VS Code API, this method becomes a trivial wrapper. public async copyFile(src: string, dest: string): Promise { const deferred = createDeferred(); const rs = this.fsExtra.createReadStream(src) @@ -285,10 +255,9 @@ export class FileSystemUtils implements IFileSystemUtils { ) { } // Create a new object using common-case default values. public static withDefaults(): FileSystemUtils { - const paths = FileSystemPaths.withDefaults(); return new FileSystemUtils( - new RawFileSystem(paths, vscode.workspace.fs, fs, fsextra), - paths, + RawFileSystem.withDefaults(), + FileSystemPaths.withDefaults(), TempFileSystem.withDefaults(), getHashString, util.promisify(glob) diff --git a/src/test/common/platform/filesystem.functional.test.ts b/src/test/common/platform/filesystem.functional.test.ts index 7451271dbcc4..1a66dc4921fe 100644 --- a/src/test/common/platform/filesystem.functional.test.ts +++ b/src/test/common/platform/filesystem.functional.test.ts @@ -415,64 +415,6 @@ suite('Raw FileSystem', () => { }); }); - suite('listdir', () => { - test('mixed', async () => { - // Create the target directory and its contents. - const dirname = await fix.createDirectory('x/y/z'); - await fix.createFile('x/y/z/__init__.py', ''); - const script = await fix.createFile('x/y/z/__main__.py', '