From c9bba3fdaef7ec710084088ea368cb013e98ebb3 Mon Sep 17 00:00:00 2001 From: HiroyukiYagihashi Date: Tue, 4 May 2021 05:33:37 +0900 Subject: [PATCH] fs: add support for async iterators to `fs.writeFile` Fixes: https://github.com/nodejs/node/issues/38075 --- doc/api/fs.md | 6 ++- lib/fs.js | 43 +++++++++++++--- lib/internal/fs/promises.js | 6 +-- lib/internal/streams/utils.js | 6 +++ test/parallel/test-fs-write-file.js | 80 +++++++++++++++++++++++++++++ 5 files changed, 127 insertions(+), 14 deletions(-) diff --git a/doc/api/fs.md b/doc/api/fs.md index 11e5b2c39fb785..ef49a3ee659745 100644 --- a/doc/api/fs.md +++ b/doc/api/fs.md @@ -3878,6 +3878,9 @@ details. * `file` {string|Buffer|URL|integer} filename or file descriptor -* `data` {string|Buffer|TypedArray|DataView|Object} +* `data` {string|Buffer|TypedArray|DataView|Object + |AsyncIterable|Iterable|Stream} * `options` {Object|string} * `encoding` {string|null} **Default:** `'utf8'` * `mode` {integer} **Default:** `0o666` diff --git a/lib/fs.js b/lib/fs.js index 310397bbed39e1..3d684468acf636 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -85,6 +85,7 @@ const { const { FSReqCallback } = binding; const { toPathIfFileURL } = require('internal/url'); const internalUtil = require('internal/util'); +const { isCustomIterable } = require('internal/streams/utils'); const { constants: { kIoMaxLength, @@ -828,12 +829,12 @@ function write(fd, buffer, offset, length, position, callback) { } else { position = length; } - length = 'utf8'; + length = length || 'utf8'; } const str = String(buffer); validateEncoding(str, length); - callback = maybeCallback(position); + callback = maybeCallback(callback || position); const req = new FSReqCallback(); req.oncomplete = wrapper; @@ -2039,7 +2040,8 @@ function lutimesSync(path, atime, mtime) { handleErrorFromBinding(ctx); } -function writeAll(fd, isUserFd, buffer, offset, length, signal, callback) { +function writeAll( + fd, isUserFd, buffer, offset, length, signal, encoding, callback) { if (signal?.aborted) { const abortError = new AbortError(); if (isUserFd) { @@ -2051,7 +2053,29 @@ function writeAll(fd, isUserFd, buffer, offset, length, signal, callback) { } return; } - // write(fd, buffer, offset, length, position, callback) + if (isCustomIterable(buffer)) { + (async () => { + for await (const buf of buffer) { + fs.write( + fd, buf, undefined, + isArrayBufferView(buf) ? buf.byteLength : encoding, + null, (writeErr, _) => { + if (writeErr) { + if (isUserFd) { + callback(writeErr); + } else { + fs.close(fd, (err) => { + callback(aggregateTwoErrors(err, writeErr)); + }); + } + } + } + ); + } + fs.close(fd, callback); + })(); + return; + } fs.write(fd, buffer, offset, length, null, (writeErr, written) => { if (writeErr) { if (isUserFd) { @@ -2070,7 +2094,8 @@ function writeAll(fd, isUserFd, buffer, offset, length, signal, callback) { } else { offset += written; length -= written; - writeAll(fd, isUserFd, buffer, offset, length, signal, callback); + writeAll( + fd, isUserFd, buffer, offset, length, signal, encoding, callback); } }); } @@ -2093,7 +2118,7 @@ function writeFile(path, data, options, callback) { options = getOptions(options, { encoding: 'utf8', mode: 0o666, flag: 'w' }); const flag = options.flag || 'w'; - if (!isArrayBufferView(data)) { + if (!isArrayBufferView(data) && !isCustomIterable(data)) { validateStringAfterArrayBufferView(data, 'data'); data = Buffer.from(String(data), options.encoding || 'utf8'); } @@ -2101,7 +2126,8 @@ function writeFile(path, data, options, callback) { if (isFd(path)) { const isUserFd = true; const signal = options.signal; - writeAll(path, isUserFd, data, 0, data.byteLength, signal, callback); + writeAll(path, isUserFd, data, + 0, data.byteLength, signal, options.encoding, callback); return; } @@ -2114,7 +2140,8 @@ function writeFile(path, data, options, callback) { } else { const isUserFd = false; const signal = options.signal; - writeAll(fd, isUserFd, data, 0, data.byteLength, signal, callback); + writeAll(fd, isUserFd, data, + 0, data.byteLength, signal, options.encoding, callback); } }); } diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js index e7f38219178952..abc14ffcc55549 100644 --- a/lib/internal/fs/promises.js +++ b/lib/internal/fs/promises.js @@ -79,7 +79,7 @@ const pathModule = require('path'); const { promisify } = require('internal/util'); const { EventEmitterMixin } = require('internal/event_target'); const { watch } = require('internal/fs/watchers'); -const { isIterable } = require('internal/streams/utils'); +const { isCustomIterable } = require('internal/streams/utils'); const kHandle = Symbol('kHandle'); const kFd = Symbol('kFd'); @@ -730,10 +730,6 @@ async function writeFile(path, data, options) { writeFileHandle(fd, data, options.signal, options.encoding), fd.close); } -function isCustomIterable(obj) { - return isIterable(obj) && !isArrayBufferView(obj) && typeof obj !== 'string'; -} - async function appendFile(path, data, options) { options = getOptions(options, { encoding: 'utf8', mode: 0o666, flag: 'a' }); options = copyObject(options); diff --git a/lib/internal/streams/utils.js b/lib/internal/streams/utils.js index b6e744250799c6..04bc76b4e31df0 100644 --- a/lib/internal/streams/utils.js +++ b/lib/internal/streams/utils.js @@ -4,6 +4,7 @@ const { SymbolAsyncIterator, SymbolIterator, } = primordials; +const { isArrayBufferView } = require('internal/util/types'); function isReadable(obj) { return !!(obj && typeof obj.pipe === 'function' && @@ -27,7 +28,12 @@ function isIterable(obj, isAsync) { typeof obj[SymbolIterator] === 'function'; } +function isCustomIterable(obj) { + return isIterable(obj) && !isArrayBufferView(obj) && typeof obj !== 'string'; +} + module.exports = { + isCustomIterable, isIterable, isReadable, isStream, diff --git a/test/parallel/test-fs-write-file.js b/test/parallel/test-fs-write-file.js index 3d0fd48f05f361..c1d3cdcddf42d8 100644 --- a/test/parallel/test-fs-write-file.js +++ b/test/parallel/test-fs-write-file.js @@ -24,6 +24,7 @@ const common = require('../common'); const assert = require('assert'); const fs = require('fs'); const join = require('path').join; +const { Readable } = require('stream'); const tmpdir = require('../common/tmpdir'); tmpdir.refresh(); @@ -95,3 +96,82 @@ fs.open(filename4, 'w+', common.mustSucceed((fd) => { process.nextTick(() => controller.abort()); } + +{ + const filenameIterable = join(tmpdir.path, 'testIterable.txt'); + const iterable = { + expected: 'abc', + *[Symbol.iterator]() { + yield 'a'; + yield 'b'; + yield 'c'; + } + }; + + fs.writeFile(filenameIterable, iterable, common.mustSucceed(() => { + const data = fs.readFileSync(filenameIterable, 'utf-8'); + assert.strictEqual(iterable.expected, data); + })); +} + +{ + const filenameBufferIterable = join(tmpdir.path, 'testBufferIterable.txt'); + const bufferIterable = { + expected: 'abc', + *[Symbol.iterator]() { + yield Buffer.from('a'); + yield Buffer.from('b'); + yield Buffer.from('c'); + } + }; + + fs.writeFile( + filenameBufferIterable, bufferIterable, common.mustSucceed(() => { + const data = fs.readFileSync(filenameBufferIterable, 'utf-8'); + assert.strictEqual(bufferIterable.expected, data); + }) + ); +} + + +{ + const filenameAsyncIterable = join(tmpdir.path, 'testAsyncIterable.txt'); + const asyncIterable = { + expected: 'abc', + *[Symbol.asyncIterator]() { + yield 'a'; + yield 'b'; + yield 'c'; + } + }; + + fs.writeFile(filenameAsyncIterable, asyncIterable, common.mustSucceed(() => { + const data = fs.readFileSync(filenameAsyncIterable, 'utf-8'); + assert.strictEqual(asyncIterable.expected, data); + })); +} + +{ + const filenameStream = join(tmpdir.path, 'testStream.txt'); + const stream = Readable.from(['a', 'b', 'c']); + const expected = 'abc'; + + fs.writeFile(filenameStream, stream, common.mustSucceed(() => { + const data = fs.readFileSync(filenameStream, 'utf-8'); + assert.strictEqual(expected, data); + })); +} + +{ + const filenameStreamWithEncoding = + join(tmpdir.path, 'testStreamWithEncoding.txt'); + const stream = Readable.from(['ümlaut', ' ', 'sechzig']); + const expected = 'ümlaut sechzig'; + + fs.writeFile( + filenameStreamWithEncoding, stream, 'latin1', common.mustSucceed(() => { + const data = fs.readFileSync(filenameStreamWithEncoding, 'latin1'); + assert.strictEqual(expected, data); + }) + ); +}