emit CREATED event for new directories

Most clients expect every new file from a new tree being added in the
watched directory to be reported. This is currently not the case if the
tree appears too quickly on disk.

This commit propagates a callback down the recursive tree building to
make sure that all files added are being reported as created to clients.

I also added an environment variable check for use when testing:
`NSFW_TEST_SLOW`. When set to `1` nsfw will artificially sleep before
crawling folders on the fs. This is to simulate the case when nsfw is
late to the party and sub-folders were created before we had time to
allocate inotify watchers. In such a case, we need to manually emit
CREATED events for new files and folders discovered by scandir.

Signed-off-by: Paul Maréchal <[email protected]>

Author: paul-marechal

binding.gyp

@@ -62,6 +62,9 @@
                 }
             }],
             ["OS=='linux' or OS=='freebsd'", {
+                "defines": [
+                    "NSFW_TEST_SLOW_<!(node -p process.env.NSFW_TEST_SLOW)"
+                ],
                 "sources": [
                     "src/linux/InotifyEventLoop.cpp",
                     "src/linux/InotifyTree.cpp",
@@ -102,7 +105,7 @@
                 ],
                 "libraries": [
                     "-L/usr/local/lib",
-                    "-linotify" 
+                    "-linotify"
                 ]
             }],
         ]

includes/linux/InotifyTree.h

@@ -10,12 +10,20 @@
 #include <vector>
 #include <map>
 #include <unordered_set>
+#include <functional>
+#include <chrono>
+#include <thread>
+
+/**
+ * void EmitCreatedEvent(std::string directory, std::string file);
+ */
+using EmitCreatedEvent = std::function<void(std::string, std::string)>;
 
 class InotifyTree {
 public:
   InotifyTree(int inotifyInstance, std::string path);
 
-  void addDirectory(int wd, std::string name);
+  void addDirectory(int wd, std::string name, EmitCreatedEvent emitCreatedEvent = nullptr);
   std::string getError();
   bool getPath(std::string &out, int wd);
   bool hasErrored();
@@ -34,10 +42,11 @@ class InotifyTree {
       InotifyNode *parent,
       std::string directory,
       std::string name,
-      ino_t inodeNumber
+      ino_t inodeNumber,
+      EmitCreatedEvent emitCreatedEvent = nullptr
     );
 
-    void addChild(std::string name);
+    void addChild(std::string name, EmitCreatedEvent emitCreatedEvent = nullptr);
     void fixPaths();
     std::string getFullPath();
     std::string getName();

js/scripts/test.js

@@ -0,0 +1,41 @@
+/* eslint-disable no-console */
+
+const cp = require('child_process');
+
+main().catch(error => {
+  console.error(error);
+  process.exitCode = 1;
+});
+
+async function main() {
+  if (process.platform !== 'win32' && process.platform !== 'darwin') {
+    // When ran as a npm script we can invoke npm bins such as node-gyp and mocha directly
+    await exec('node-gyp rebuild', { env: { ...process.env, NSFW_TEST_SLOW: 1 } });
+    await exec('mocha --expose-gc js/spec/index-slow-spec.js');
+    await exec('node-gyp rebuild');
+  }
+  await exec('mocha --expose-gc js/spec/index-spec.js');
+}
+
+/**
+ * @param {string} commandline ...
+ * @param {cp.SpawnOptions} options ...
+ * @returns {Promise<void>} ...
+ */
+function exec(commandline, options = {}) {
+  return new Promise((resolve, reject) => {
+    const command = cp.spawn(commandline, {
+      shell: true,
+      stdio: 'inherit',
+      ...options
+    });
+    command.on('error', reject);
+    command.on('exit', (code, signal) => {
+      if (code === 0) {
+        resolve();
+      } else {
+        reject(new Error(`${JSON.stringify(commandline)} exited with ${signal || code}`));
+      }
+    });
+  });
+}

js/spec/common.js

@@ -0,0 +1,8 @@
+const path = require('path');
+const util = require('util');
+
+exports.DEBOUNCE = 1000;
+exports.TIMEOUT_PER_STEP = 3000;
+exports.WORKDIR = path.resolve(__dirname, '../../mockfs');
+
+exports.sleep = util.promisify(setTimeout);

js/spec/index-slow-spec.js

@@ -0,0 +1,24 @@
+const assert = require('assert');
+const fse = require('fs-extra');
+
+const { WORKDIR: workDir } = require('./common');
+
+const nestedFileTest = require('./nested-file-test');
+
+const nsfw = require('../src');
+
+describe('Node Sentinel File Watcher (slow)', function() {
+  this.timeout(120000);
+
+  assert.ok(nsfw._native.NSFW_TEST_SLOW === true, 'NSFW should be built in slow mode');
+
+  beforeEach(function() {
+    return fse.mkdir(workDir, { recursive: true });
+  });
+
+  afterEach(function() {
+    return fse.remove(workDir);
+  });
+
+  nestedFileTest();
+});

js/spec/index-spec.js

@@ -2,19 +2,17 @@ const assert = require('assert');
 const exec = require('executive');
 const fse = require('fs-extra');
 const path = require('path');
-const { promisify } = require('util');
 
-const nsfw = require('../src/');
+const { DEBOUNCE, TIMEOUT_PER_STEP, WORKDIR: workDir, sleep } = require('./common');
 
-const DEBOUNCE = 1000;
-const TIMEOUT_PER_STEP = 3000;
+const nestedFileTest = require('./nested-file-test');
 
-const sleep = promisify(setTimeout);
+const nsfw = require('../src');
 
 describe('Node Sentinel File Watcher', function() {
   this.timeout(120000);
 
-  const workDir = path.resolve('./mockfs');
+  assert.ok(typeof nsfw._native.NSFW_TEST_SLOW === 'undefined', 'NSFW should NOT be built in slow mode');
 
   beforeEach(async function() {
     async function makeDir(identifier) {
@@ -438,49 +436,7 @@ describe('Node Sentinel File Watcher', function() {
   });
 
   describe('Recursive', function() {
-    it('can listen for the creation of a deeply nested file', async function() {
-      const folders = 'a_very_deep_tree'.split(''); // ['a', '_', ...]
-      const file = 'a_file.txt';
-
-      const events = new Map();
-      let directory = workDir; for (const folder of folders) {
-        events.set(directory, { file: folder, found: false });
-        directory = path.join(directory, folder);
-      }
-      events.set(directory, { file, found: false });
-
-      function findEvent(element) {
-        if (element.action === nsfw.actions.CREATED) {
-          const item = events.get(element.directory);
-          if (item === undefined || item.file !== element.file) {
-            return;
-          }
-          item.found = true;
-        }
-      }
-
-      let watch = await nsfw(
-        workDir,
-        events => events.forEach(findEvent),
-        { debounceMS: DEBOUNCE }
-      );
-
-      try {
-        await watch.start();
-        await sleep(TIMEOUT_PER_STEP);
-        await fse.mkdirp(directory);
-        await fse.close(await fse.open(path.join(directory, file), 'w'));
-        await sleep(TIMEOUT_PER_STEP);
-
-        for (const [directory, item] of events.entries()) {
-          assert.ok(item.found, `${path.join(directory, item.file)} wasn't found`);
-        }
-      } finally {
-        await watch.stop();
-        watch = null;
-      }
-    });
-
+    nestedFileTest();
     it('can listen for the destruction of a directory and its subtree', async function() {
       const inPath = path.resolve(workDir, 'test4');
       let deletionCount = 0;

js/spec/nested-file-test.js

@@ -0,0 +1,73 @@
+const path = require('path');
+const assert = require('assert');
+const fse = require('fs-extra');
+
+const { DEBOUNCE, TIMEOUT_PER_STEP, WORKDIR: workDir, sleep } = require('./common');
+
+const nsfw = require('../src');
+
+/**
+ * Run this function to register the test
+ * @returns {void}
+ */
+module.exports = function() {
+  it('can listen for the creation of a deeply nested file', async function () {
+    const folders = 'a_very_deep_tree'.split(''); // ['a', '_', ...]
+    const file = 'a_file.txt';
+
+    // Resolve a promise as soon as all events have been found
+    let done; const promise = new Promise(resolve => {
+      done = resolve;
+    });
+
+    /**
+     * We will remove or pop entries as we find them.
+     * This set should be empty at the end of the test.
+     * @type {Set<string>}
+     */
+    const events = new Set();
+    let directory = workDir; for (const folder of folders) {
+      directory = path.join(directory, folder);
+      events.add(directory);
+    }
+    const filePath = path.join(directory, file);
+    events.add(filePath);
+
+    function findEvent(element) {
+      if (element.action === nsfw.actions.CREATED) {
+        const file = path.join(element.directory, element.file);
+        if (events.has(file)) {
+          events.delete(file);
+          if (events.size === 0) {
+            done();
+          }
+        }
+      }
+    }
+
+    let watch = await nsfw(
+      workDir,
+      events => events.forEach(findEvent),
+      { debounceMS: DEBOUNCE }
+    );
+
+    try {
+      await watch.start();
+      await sleep(TIMEOUT_PER_STEP);
+      await fse.mkdirp(directory);
+      await fse.close(await fse.open(filePath, 'w'));
+      await Promise.race([
+        sleep(folders.length * 500 * 1.5),
+        promise,
+      ]);
+
+      // Make sure that we got the events for each nested directory
+      assert.ok(events.size === 0, `Some files were not detected:\n${
+        Array.from(events, file => `- ${file}`).join('\n')
+      }`);
+    } finally {
+      await watch.stop();
+      watch = null;
+    }
+  });
+};

js/src/index.js

@@ -102,6 +102,8 @@ nsfw.actions = {
   RENAMED: 3
 };
 
+nsfw._native = NSFW;
+
 if (NSFW.getAllocatedInstanceCount) {
   nsfw.getAllocatedInstanceCount = NSFW.getAllocatedInstanceCount;
 }

package.json

@@ -5,7 +5,7 @@
   "main": "js/src/index.js",
   "scripts": {
     "lint": "eslint js/src js/spec",
-    "test": "yarn lint && mocha --expose-gc js/spec"
+    "test": "yarn lint && node js/scripts/test.js"
   },
   "repository": {
     "type": "git",

src/NSFW.cpp

@@ -339,6 +339,13 @@ Napi::Object NSFW::Init(Napi::Env env, Napi::Object exports) {
     ));
   }
 
+  #ifdef NSFW_TEST_SLOW_1
+    nsfwConstructor.DefineProperty(Napi::PropertyDescriptor::Value(
+      "NSFW_TEST_SLOW",
+      Napi::Boolean::New(env, true)
+    ));
+  #endif
+
   constructor = Napi::Persistent(nsfwConstructor);
   constructor.SuppressDestruct();
 

src/linux/InotifyService.cpp

@@ -98,8 +98,10 @@ void InotifyService::createDirectory(int wd, std::string name) {
     return;
   }
 
-  mTree->addDirectory(wd, name);
   dispatch(CREATED, wd, name);
+  mTree->addDirectory(wd, name, [this](std::string directory, std::string file) {
+    mQueue->enqueue(CREATED, directory, file);
+  });
 }
 
 void InotifyService::removeDirectory(int wd) {