From cbd8d715b2286e5726e6988921f5c870cbf74127 Mon Sep 17 00:00:00 2001 From: Jeremiah Senkpiel Date: Tue, 27 Aug 2019 17:14:27 -0700 Subject: fs: introduce `opendir()` and `fs.Dir` This adds long-requested methods for asynchronously interacting and iterating through directory entries by using `uv_fs_opendir`, `uv_fs_readdir`, and `uv_fs_closedir`. `fs.opendir()` and friends return an `fs.Dir`, which contains methods for doing reads and cleanup. `fs.Dir` also has the async iterator symbol exposed. The `read()` method and friends only return `fs.Dirent`s for this API. Having a entry type or doing a `stat` call is deemed to be necessary in the majority of cases, so just returning dirents seems like the logical choice for a new api. Reading when there are no more entries returns `null` instead of a dirent. However the async iterator hides that (and does automatic cleanup). The code lives in separate files from the rest of fs, this is done partially to prevent over-pollution of those (already very large) files, but also in the case of js allows loading into `fsPromises`. Due to async_hooks, this introduces a new handle type of `DIRHANDLE`. This PR does not attempt to make complete optimization of this feature. Notable future improvements include: - Moving promise work into C++ land like FileHandle. - Possibly adding `readv()` to do multi-entry directory reads. - Aliasing `fs.readdir` to `fs.scandir` and doing a deprecation. Refs: https://github.com/nodejs/node-v0.x-archive/issues/388 Refs: https://github.com/nodejs/node/issues/583 Refs: https://github.com/libuv/libuv/pull/2057 PR-URL: https://github.com/nodejs/node/pull/29349 Reviewed-By: Anna Henningsen Reviewed-By: David Carlier --- test/parallel/test-bootstrap-modules.js | 2 + test/parallel/test-fs-opendir.js | 174 ++++++++++++++++++++++++++ test/sequential/test-async-wrap-getasyncid.js | 7 ++ 3 files changed, 183 insertions(+) create mode 100644 test/parallel/test-fs-opendir.js (limited to 'test') diff --git a/test/parallel/test-bootstrap-modules.js b/test/parallel/test-bootstrap-modules.js index ae69f5aef0..7ca48c49cb 100644 --- a/test/parallel/test-bootstrap-modules.js +++ b/test/parallel/test-bootstrap-modules.js @@ -17,6 +17,7 @@ const expectedModules = new Set([ 'Internal Binding contextify', 'Internal Binding credentials', 'Internal Binding fs', + 'Internal Binding fs_dir', 'Internal Binding inspector', 'Internal Binding module_wrap', 'Internal Binding native_module', @@ -42,6 +43,7 @@ const expectedModules = new Set([ 'NativeModule internal/encoding', 'NativeModule internal/errors', 'NativeModule internal/fixed_queue', + 'NativeModule internal/fs/dir', 'NativeModule internal/fs/utils', 'NativeModule internal/idna', 'NativeModule internal/linkedlist', diff --git a/test/parallel/test-fs-opendir.js b/test/parallel/test-fs-opendir.js new file mode 100644 index 0000000000..c9a6d657ed --- /dev/null +++ b/test/parallel/test-fs-opendir.js @@ -0,0 +1,174 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const fs = require('fs'); +const path = require('path'); + +const tmpdir = require('../common/tmpdir'); + +const testDir = tmpdir.path; +const files = ['empty', 'files', 'for', 'just', 'testing']; + +// Make sure tmp directory is clean +tmpdir.refresh(); + +// Create the necessary files +files.forEach(function(filename) { + fs.closeSync(fs.openSync(path.join(testDir, filename), 'w')); +}); + +function assertDirent(dirent) { + assert(dirent instanceof fs.Dirent); + assert.strictEqual(dirent.isFile(), true); + assert.strictEqual(dirent.isDirectory(), false); + assert.strictEqual(dirent.isSocket(), false); + assert.strictEqual(dirent.isBlockDevice(), false); + assert.strictEqual(dirent.isCharacterDevice(), false); + assert.strictEqual(dirent.isFIFO(), false); + assert.strictEqual(dirent.isSymbolicLink(), false); +} + +const dirclosedError = { + code: 'ERR_DIR_CLOSED' +}; + +// Check the opendir Sync version +{ + const dir = fs.opendirSync(testDir); + const entries = files.map(() => { + const dirent = dir.readSync(); + assertDirent(dirent); + return dirent.name; + }); + assert.deepStrictEqual(files, entries.sort()); + + // dir.read should return null when no more entries exist + assert.strictEqual(dir.readSync(), null); + + // check .path + assert.strictEqual(dir.path, testDir); + + dir.closeSync(); + + assert.throws(() => dir.readSync(), dirclosedError); + assert.throws(() => dir.closeSync(), dirclosedError); +} + +// Check the opendir async version +fs.opendir(testDir, common.mustCall(function(err, dir) { + assert.ifError(err); + dir.read(common.mustCall(function(err, dirent) { + assert.ifError(err); + + // Order is operating / file system dependent + assert(files.includes(dirent.name), `'files' should include ${dirent}`); + assertDirent(dirent); + + dir.close(common.mustCall(function(err) { + assert.ifError(err); + })); + })); +})); + +// opendir() on file should throw ENOTDIR +assert.throws(function() { + fs.opendirSync(__filename); +}, /Error: ENOTDIR: not a directory/); + +fs.opendir(__filename, common.mustCall(function(e) { + assert.strictEqual(e.code, 'ENOTDIR'); +})); + +[false, 1, [], {}, null, undefined].forEach((i) => { + common.expectsError( + () => fs.opendir(i, common.mustNotCall()), + { + code: 'ERR_INVALID_ARG_TYPE', + type: TypeError + } + ); + common.expectsError( + () => fs.opendirSync(i), + { + code: 'ERR_INVALID_ARG_TYPE', + type: TypeError + } + ); +}); + +// Promise-based tests +async function doPromiseTest() { + // Check the opendir Promise version + const dir = await fs.promises.opendir(testDir); + const entries = []; + + let i = files.length; + while (i--) { + const dirent = await dir.read(); + entries.push(dirent.name); + assertDirent(dirent); + } + + assert.deepStrictEqual(files, entries.sort()); + + // dir.read should return null when no more entries exist + assert.strictEqual(await dir.read(), null); + + await dir.close(); +} +doPromiseTest().then(common.mustCall()); + +// Async iterator +async function doAsyncIterTest() { + const entries = []; + for await (const dirent of await fs.promises.opendir(testDir)) { + entries.push(dirent.name); + assertDirent(dirent); + } + + assert.deepStrictEqual(files, entries.sort()); + + // Automatically closed during iterator +} +doAsyncIterTest().then(common.mustCall()); + +// Async iterators should do automatic cleanup + +async function doAsyncIterBreakTest() { + const dir = await fs.promises.opendir(testDir); + for await (const dirent of dir) { // eslint-disable-line no-unused-vars + break; + } + + await assert.rejects(async () => dir.read(), dirclosedError); +} +doAsyncIterBreakTest().then(common.mustCall()); + +async function doAsyncIterReturnTest() { + const dir = await fs.promises.opendir(testDir); + await (async function() { + for await (const dirent of dir) { // eslint-disable-line no-unused-vars + return; + } + })(); + + await assert.rejects(async () => dir.read(), dirclosedError); +} +doAsyncIterReturnTest().then(common.mustCall()); + +async function doAsyncIterThrowTest() { + const dir = await fs.promises.opendir(testDir); + try { + for await (const dirent of dir) { // eslint-disable-line no-unused-vars + throw new Error('oh no'); + } + } catch (err) { + if (err.message !== 'oh no') { + throw err; + } + } + + await assert.rejects(async () => dir.read(), dirclosedError); +} +doAsyncIterThrowTest().then(common.mustCall()); diff --git a/test/sequential/test-async-wrap-getasyncid.js b/test/sequential/test-async-wrap-getasyncid.js index bef6b050ff..cc823cc17f 100644 --- a/test/sequential/test-async-wrap-getasyncid.js +++ b/test/sequential/test-async-wrap-getasyncid.js @@ -306,3 +306,10 @@ if (process.features.inspector && common.isMainThread) { { v8.getHeapSnapshot().destroy(); } + +// DIRHANDLE +{ + const dirBinding = internalBinding('fs_dir'); + const handle = dirBinding.opendir('./', 'utf8', undefined, {}); + testInitialized(handle, 'DirHandle'); +} -- cgit v1.2.3