From 35164fa46633ce58cda2a40ffe519e6725b2872c Mon Sep 17 00:00:00 2001
From: Colin Ihrig <cjihrig@gmail.com>
Date: Tue, 17 Oct 2023 21:51:41 -0400
Subject: [PATCH] fs: add flush option to appendFile() functions

This commit adds documentation and tests for the 'flush' option
of the fs.appendFile family of functions. Technically, support
was indirectly added in #50009, but this makes it more official.

Refs: https://github.com/nodejs/node/issues/49886
Refs: https://github.com/nodejs/node/pull/50009
PR-URL: https://github.com/nodejs/node/pull/50095
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
---
 doc/api/fs.md                              |  21 ++++
 test/parallel/test-fs-append-file-flush.js | 114 +++++++++++++++++++++
 2 files changed, 135 insertions(+)
 create mode 100644 test/parallel/test-fs-append-file-flush.js

diff --git a/doc/api/fs.md b/doc/api/fs.md
index 8442fa2a5a18b4..7a544a59593e08 100644
--- a/doc/api/fs.md
+++ b/doc/api/fs.md
@@ -180,6 +180,9 @@ longer be used.
 <!-- YAML
 added: v10.0.0
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/50095
+    description: The `flush` option is now supported.
   - version:
       - v15.14.0
       - v14.18.0
@@ -194,6 +197,8 @@ changes:
 * `data` {string|Buffer|TypedArray|DataView|AsyncIterable|Iterable|Stream}
 * `options` {Object|string}
   * `encoding` {string|null} **Default:** `'utf8'`
+  * `flush` {boolean} If `true`, the underlying file descriptor is flushed
+    prior to closing it. **Default:** `false`.
 * Returns: {Promise} Fulfills with `undefined` upon success.
 
 Alias of [`filehandle.writeFile()`][].
@@ -878,6 +883,10 @@ the error raised if the file is not accessible.
 
 <!-- YAML
 added: v10.0.0
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/50095
+    description: The `flush` option is now supported.
 -->
 
 * `path` {string|Buffer|URL|FileHandle} filename or {FileHandle}
@@ -886,6 +895,8 @@ added: v10.0.0
   * `encoding` {string|null} **Default:** `'utf8'`
   * `mode` {integer} **Default:** `0o666`
   * `flag` {string} See [support of file system `flags`][]. **Default:** `'a'`.
+  * `flush` {boolean} If `true`, the underlying file descriptor is flushed
+    prior to closing it. **Default:** `false`.
 * Returns: {Promise} Fulfills with `undefined` upon success.
 
 Asynchronously append data to a file, creating the file if it does not yet
@@ -2028,6 +2039,9 @@ the user from reading or writing to it.
 <!-- YAML
 added: v0.6.7
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/50095
+    description: The `flush` option is now supported.
   - version: v18.0.0
     pr-url: https://github.com/nodejs/node/pull/41678
     description: Passing an invalid callback to the `callback` argument
@@ -2055,6 +2069,8 @@ changes:
   * `encoding` {string|null} **Default:** `'utf8'`
   * `mode` {integer} **Default:** `0o666`
   * `flag` {string} See [support of file system `flags`][]. **Default:** `'a'`.
+  * `flush` {boolean} If `true`, the underlying file descriptor is flushed
+    prior to closing it. **Default:** `false`.
 * `callback` {Function}
   * `err` {Error}
 
@@ -5112,6 +5128,9 @@ try {
 <!-- YAML
 added: v0.6.7
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/50095
+    description: The `flush` option is now supported.
   - version: v7.0.0
     pr-url: https://github.com/nodejs/node/pull/7831
     description: The passed `options` object will never be modified.
@@ -5126,6 +5145,8 @@ changes:
   * `encoding` {string|null} **Default:** `'utf8'`
   * `mode` {integer} **Default:** `0o666`
   * `flag` {string} See [support of file system `flags`][]. **Default:** `'a'`.
+  * `flush` {boolean} If `true`, the underlying file descriptor is flushed
+    prior to closing it. **Default:** `false`.
 
 Synchronously append data to a file, creating the file if it does not yet
 exist. `data` can be a string or a {Buffer}.
diff --git a/test/parallel/test-fs-append-file-flush.js b/test/parallel/test-fs-append-file-flush.js
new file mode 100644
index 00000000000000..69deeb4e8f9d14
--- /dev/null
+++ b/test/parallel/test-fs-append-file-flush.js
@@ -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.appendFileSync(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.appendFileSync(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.appendFileSync(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.appendFileSync(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.appendFile(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.appendFile(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.appendFile(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.appendFile(file, data, { flush: v });
+      assert.strictEqual(await fsp.readFile(file, 'utf8'), data);
+    }
+  });
+});