fix: improved watch logic of customDevices #670 (#675)

lib/Gateway.js

@@ -5,6 +5,7 @@
 'use strict'
 
 const fs = require('fs')
+const path = require('path')
 const reqlib = require('app-root-path').require
 const utils = reqlib('/lib/utils.js')
 const EventEmitter = require('events')
@@ -24,21 +25,75 @@ var allDevices = hassDevices // will contain customDevices + hassDevices
 
 debug.color = 2
 
-// util function to watch for file changes, prevents the watch to be called twice
-const watch = (path, fn) => {
-  var lock = false
-  fs.watch(path, function () {
-    if (lock) return
-
-    lock = true
-    fn()
-    setTimeout(() => {
-      lock = false
-    }, 1000)
-  })
+// watcher initiates a watch on a file. if this fails (e.g., because the file
+// doesn't exist), instead watch the directory. If the directory watch
+// triggers, cancel it and try to watch the file again. Meanwhile spam `fn()`
+// on any change, trusting that it's idempotent.
+var watchers = new Map()
+const watch = (filename, fn) => {
+  try {
+    watchers.set(
+      filename,
+      fs.watch(filename, e => {
+        fn()
+        if (e === 'rename') {
+          watchers.get(filename).close()
+          watch(filename, fn)
+        }
+      })
+    )
+  } catch {
+    watchers.set(
+      filename,
+      fs.watch(path.dirname(filename), () => {
+        watchers.get(filename).close()
+        watch(filename, fn)
+        fn()
+      })
+    )
+  }
 }
 
-function loadCustomDevices (devices) {
+const customDevicesJsPath = utils.joinPath(true, CUSTOM_DEVICES) + '.js'
+const customDevicesJsonPath = utils.joinPath(true, CUSTOM_DEVICES) + '.json'
+
+var lastCustomDevicesLoad = null
+// loadCustomDevices attempts to load a custom devices file, preferring `.js`
+// but falling back to `.json` only if a `.js` file does not exist. It stores
+// a sha of the loaded data, and will skip re-loading any time the data has
+// not changed.
+const loadCustomDevices = () => {
+  var loaded = ''
+  var devices = null
+
+  try {
+    if (fs.existsSync(customDevicesJsPath)) {
+      loaded = customDevicesJsPath
+      devices = reqlib(CUSTOM_DEVICES)
+    } else if (fs.existsSync(customDevicesJsonPath)) {
+      loaded = customDevicesJsonPath
+      devices = JSON.parse(fs.readFileSync(loaded))
+    } else {
+      debug('no custom devices sources found')
+      return
+    }
+  } catch (error) {
+    debug('failed to load ' + loaded + ':', error)
+    return
+  }
+
+  const sha = require('crypto')
+    .createHash('sha256')
+    .update(JSON.stringify(devices))
+    .digest('hex')
+  if (lastCustomDevicesLoad === sha) {
+    return
+  }
+
+  debug('loading custom devices from', loaded)
+
+  lastCustomDevicesLoad = sha
+
   allDevices = Object.assign({}, hassDevices, devices)
   debug(
     'Loaded',
@@ -47,27 +102,9 @@ function loadCustomDevices (devices) {
   )
 }
 
-try {
-  const devices = reqlib(CUSTOM_DEVICES)
-  loadCustomDevices(devices)
-  CUSTOM_DEVICES = utils.joinPath(true, CUSTOM_DEVICES)
-  if (fs.existsSync(CUSTOM_DEVICES + '.json')) {
-    watch(CUSTOM_DEVICES + '.json', function () {
-      try {
-        const fileContent = fs.readFileSync(CUSTOM_DEVICES + '.json')
-        loadCustomDevices(JSON.parse(fileContent))
-      } catch (error) {
-        debug('Error while reading', CUSTOM_DEVICES + '.json', error.message)
-      }
-    })
-  }
-} catch (error) {
-  if (error.message.indexOf('Cannot find module') >= 0) {
-    debug('No customDevices file found')
-  } else {
-    debug('Error while parsing customDevices file:', error.message)
-  }
-}
+loadCustomDevices()
+watch(customDevicesJsPath, 0, loadCustomDevices)
+watch(customDevicesJsonPath, 1, loadCustomDevices)
 
 /**
  * The constructor

test/lib/Gateway.test.js

@@ -57,4 +57,12 @@ describe('#Gateway', () => {
       })
     })
   })
+
+  afterEach(() => {
+    mod.__get__('watchers').forEach(v => {
+      if (v != null) {
+        v.close()
+      }
+    })
+  })
 })