Skip to content

Commit

Permalink
fs: add flush option to writeFile() functions
Browse files Browse the repository at this point in the history
This commit adds a 'flush' option to the fs.writeFile family of
functions.

Refs: #49886
PR-URL: #50009
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: LiviaMedeiros <[email protected]>
Reviewed-By: Daijiro Wachi <[email protected]>
  • Loading branch information
cjihrig authored Oct 4, 2023
1 parent 557044a commit e01c1d7
Show file tree
Hide file tree
Showing 4 changed files with 210 additions and 14 deletions.
20 changes: 18 additions & 2 deletions doc/api/fs.md
Original file line number Diff line number Diff line change
Expand Up @@ -1742,6 +1742,9 @@ All the [caveats][] for `fs.watch()` also apply to `fsPromises.watch()`.
<!-- YAML
added: v10.0.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/50009
description: The `flush` option is now supported.
- version:
- v15.14.0
- v14.18.0
Expand All @@ -1765,6 +1768,9 @@ changes:
* `encoding` {string|null} **Default:** `'utf8'`
* `mode` {integer} **Default:** `0o666`
* `flag` {string} See [support of file system `flags`][]. **Default:** `'w'`.
* `flush` {boolean} If all data is successfully written to the file, and
`flush` is `true`, `filehandle.sync()` is used to flush the data.
**Default:** `false`.
* `signal` {AbortSignal} allows aborting an in-progress writeFile
* Returns: {Promise} Fulfills with `undefined` upon success.
Expand Down Expand Up @@ -4879,6 +4885,9 @@ details.
<!-- YAML
added: v0.1.29
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/50009
description: The `flush` option is now supported.
- version: v19.0.0
pr-url: https://github.com/nodejs/node/pull/42796
description: Passing to the `string` parameter an object with an own
Expand Down Expand Up @@ -4936,6 +4945,9 @@ changes:
* `encoding` {string|null} **Default:** `'utf8'`
* `mode` {integer} **Default:** `0o666`
* `flag` {string} See [support of file system `flags`][]. **Default:** `'w'`.
* `flush` {boolean} If all data is successfully written to the file, and
`flush` is `true`, `fs.fsync()` is used to flush the data.
**Default:** `false`.
* `signal` {AbortSignal} allows aborting an in-progress writeFile
* `callback` {Function}
* `err` {Error|AggregateError}
Expand Down Expand Up @@ -6167,6 +6179,9 @@ this API: [`fs.utimes()`][].
<!-- YAML
added: v0.1.29
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/50009
description: The `flush` option is now supported.
- version: v19.0.0
pr-url: https://github.com/nodejs/node/pull/42796
description: Passing to the `data` parameter an object with an own
Expand Down Expand Up @@ -6201,8 +6216,9 @@ changes:
* `encoding` {string|null} **Default:** `'utf8'`
* `mode` {integer} **Default:** `0o666`
* `flag` {string} See [support of file system `flags`][]. **Default:** `'w'`.

Returns `undefined`.
* `flush` {boolean} If all data is successfully written to the file, and
`flush` is `true`, `fs.fsyncSync()` is used to flush the data.
Returns `undefined`.

The `mode` option only affects the newly created file. See [`fs.open()`][]
for more details.
Expand Down
59 changes: 50 additions & 9 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -2216,7 +2216,7 @@ function lutimesSync(path, atime, mtime) {
handleErrorFromBinding(ctx);
}

function writeAll(fd, isUserFd, buffer, offset, length, signal, callback) {
function writeAll(fd, isUserFd, buffer, offset, length, signal, flush, callback) {
if (signal?.aborted) {
const abortError = new AbortError(undefined, { cause: signal?.reason });
if (isUserFd) {
Expand All @@ -2239,15 +2239,33 @@ function writeAll(fd, isUserFd, buffer, offset, length, signal, callback) {
});
}
} else if (written === length) {
if (isUserFd) {
callback(null);
if (!flush) {
if (isUserFd) {
callback(null);
} else {
fs.close(fd, callback);
}
} else {
fs.close(fd, callback);
fs.fsync(fd, (syncErr) => {
if (syncErr) {
if (isUserFd) {
callback(syncErr);
} else {
fs.close(fd, (err) => {
callback(aggregateTwoErrors(err, syncErr));
});
}
} else if (isUserFd) {
callback(null);
} else {
fs.close(fd, callback);
}
});
}
} else {
offset += written;
length -= written;
writeAll(fd, isUserFd, buffer, offset, length, signal, callback);
writeAll(fd, isUserFd, buffer, offset, length, signal, flush, callback);
}
});
}
Expand All @@ -2261,14 +2279,23 @@ function writeAll(fd, isUserFd, buffer, offset, length, signal, callback) {
* mode?: number;
* flag?: string;
* signal?: AbortSignal;
* flush?: boolean;
* } | string} [options]
* @param {(err?: Error) => any} callback
* @returns {void}
*/
function writeFile(path, data, options, callback) {
callback = maybeCallback(callback || options);
options = getOptions(options, { encoding: 'utf8', mode: 0o666, flag: 'w' });
options = getOptions(options, {
encoding: 'utf8',
mode: 0o666,
flag: 'w',
flush: false,
});
const flag = options.flag || 'w';
const flush = options.flush ?? false;

validateBoolean(flush, 'options.flush');

if (!isArrayBufferView(data)) {
validateStringAfterArrayBufferView(data, 'data');
Expand All @@ -2278,7 +2305,7 @@ 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, flush, callback);
return;
}

Expand All @@ -2291,7 +2318,7 @@ 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, flush, callback);
}
});
}
Expand All @@ -2304,11 +2331,21 @@ function writeFile(path, data, options, callback) {
* encoding?: string | null;
* mode?: number;
* flag?: string;
* flush?: boolean;
* } | string} [options]
* @returns {void}
*/
function writeFileSync(path, data, options) {
options = getOptions(options, { encoding: 'utf8', mode: 0o666, flag: 'w' });
options = getOptions(options, {
encoding: 'utf8',
mode: 0o666,
flag: 'w',
flush: false,
});

const flush = options.flush ?? false;

validateBoolean(flush, 'options.flush');

if (!isArrayBufferView(data)) {
validateStringAfterArrayBufferView(data, 'data');
Expand All @@ -2328,6 +2365,10 @@ function writeFileSync(path, data, options) {
offset += written;
length -= written;
}

if (flush) {
fs.fsyncSync(fd);
}
} finally {
if (!isUserFd) fs.closeSync(fd);
}
Expand Down
31 changes: 28 additions & 3 deletions lib/internal/fs/promises.js
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,18 @@ async function handleFdClose(fileOpPromise, closeFunc) {
);
}

async function handleFdSync(fileOpPromise, handle) {
return PromisePrototypeThen(
fileOpPromise,
(result) => PromisePrototypeThen(
handle.sync(),
() => result,
(syncError) => PromiseReject(syncError),
),
(opError) => PromiseReject(opError),
);
}

async function fsCall(fn, handle, ...args) {
assert(handle[kRefs] !== undefined,
'handle must be an instance of FileHandle');
Expand Down Expand Up @@ -1007,8 +1019,16 @@ async function mkdtemp(prefix, options) {
}

async function writeFile(path, data, options) {
options = getOptions(options, { encoding: 'utf8', mode: 0o666, flag: 'w' });
options = getOptions(options, {
encoding: 'utf8',
mode: 0o666,
flag: 'w',
flush: false,
});
const flag = options.flag || 'w';
const flush = options.flush ?? false;

validateBoolean(flush, 'options.flush');

if (!isArrayBufferView(data) && !isCustomIterable(data)) {
validateStringAfterArrayBufferView(data, 'data');
Expand All @@ -1022,8 +1042,13 @@ async function writeFile(path, data, options) {
checkAborted(options.signal);

const fd = await open(path, flag, options.mode);
return handleFdClose(
writeFileHandle(fd, data, options.signal, options.encoding), fd.close);
let writeOp = writeFileHandle(fd, data, options.signal, options.encoding);

if (flush) {
writeOp = handleFdSync(writeOp, fd);
}

return handleFdClose(writeOp, fd.close);
}

function isCustomIterable(obj) {
Expand Down
114 changes: 114 additions & 0 deletions test/parallel/test-fs-write-file-flush.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
'use strict';
const common = require('../common');
const tmpdir = require('../common/tmpdir');
const assert = require('node:assert');
const fs = require('node:fs');
const fsp = require('node:fs/promises');
const test = require('node:test');
const data = 'foo';
let cnt = 0;

function nextFile() {
return tmpdir.resolve(`${cnt++}.out`);
}

tmpdir.refresh();

test('synchronous version', async (t) => {
await t.test('validation', (t) => {
for (const v of ['true', '', 0, 1, [], {}, Symbol()]) {
assert.throws(() => {
fs.writeFileSync(nextFile(), data, { flush: v });
}, { code: 'ERR_INVALID_ARG_TYPE' });
}
});

await t.test('performs flush', (t) => {
const spy = t.mock.method(fs, 'fsyncSync');
const file = nextFile();
fs.writeFileSync(file, data, { flush: true });
const calls = spy.mock.calls;
assert.strictEqual(calls.length, 1);
assert.strictEqual(calls[0].result, undefined);
assert.strictEqual(calls[0].error, undefined);
assert.strictEqual(calls[0].arguments.length, 1);
assert.strictEqual(typeof calls[0].arguments[0], 'number');
assert.strictEqual(fs.readFileSync(file, 'utf8'), data);
});

await t.test('does not perform flush', (t) => {
const spy = t.mock.method(fs, 'fsyncSync');

for (const v of [undefined, null, false]) {
const file = nextFile();
fs.writeFileSync(file, data, { flush: v });
assert.strictEqual(fs.readFileSync(file, 'utf8'), data);
}

assert.strictEqual(spy.mock.calls.length, 0);
});
});

test('callback version', async (t) => {
await t.test('validation', (t) => {
for (const v of ['true', '', 0, 1, [], {}, Symbol()]) {
assert.throws(() => {
fs.writeFileSync(nextFile(), data, { flush: v });
}, { code: 'ERR_INVALID_ARG_TYPE' });
}
});

await t.test('performs flush', (t, done) => {
const spy = t.mock.method(fs, 'fsync');
const file = nextFile();
fs.writeFile(file, data, { flush: true }, common.mustSucceed(() => {
const calls = spy.mock.calls;
assert.strictEqual(calls.length, 1);
assert.strictEqual(calls[0].result, undefined);
assert.strictEqual(calls[0].error, undefined);
assert.strictEqual(calls[0].arguments.length, 2);
assert.strictEqual(typeof calls[0].arguments[0], 'number');
assert.strictEqual(typeof calls[0].arguments[1], 'function');
assert.strictEqual(fs.readFileSync(file, 'utf8'), data);
done();
}));
});

await t.test('does not perform flush', (t, done) => {
const values = [undefined, null, false];
const spy = t.mock.method(fs, 'fsync');
let cnt = 0;

for (const v of values) {
const file = nextFile();

fs.writeFile(file, data, { flush: v }, common.mustSucceed(() => {
assert.strictEqual(fs.readFileSync(file, 'utf8'), data);
cnt++;

if (cnt === values.length) {
assert.strictEqual(spy.mock.calls.length, 0);
done();
}
}));
}
});
});

test('promise based version', async (t) => {
await t.test('validation', async (t) => {
for (const v of ['true', '', 0, 1, [], {}, Symbol()]) {
await assert.rejects(() => {
return fsp.writeFile(nextFile(), data, { flush: v });
}, { code: 'ERR_INVALID_ARG_TYPE' });
}
});

await t.test('success path', async (t) => {
for (const v of [undefined, null, false, true]) {
const file = nextFile();
await fsp.writeFile(file, data, { flush: v });
assert.strictEqual(await fsp.readFile(file, 'utf8'), data);
}
});
});

0 comments on commit e01c1d7

Please sign in to comment.