Refactor

.eslintrc

@@ -38,6 +38,7 @@
     "radix": "error",
     "require-await": "error",
     "require-atomic-updates": "error",
+    "semi": ["error", "always"],
     "use-isnan": "error",
     "valid-typeof": "error",
     "jsdoc/check-alignment": 1,

.gitignore

-node_modules
-site-config/*
-!site-config/*.EXAMPLE
+# Ignore everything
+*
+# Include necessary dot-files
+!.eslintignore
+!.eslintrc
+!.gitignore
+# Include necessary files
+!package.json
+!package-lock.json
+!LICENSE
+!README.md
+# Include necessary directories
+!lib
+!lib/*.js
+!site/config
+!site/config/*.EXAMPLE

README.md

@@ -4,20 +4,13 @@ Download files from IQ SFTP server to local filesystem.
 
 * Used for BO report delivery to Rec Fusion customers.
 * BO -> SFTP [project documentation](https://iqops.dsc.umich.edu/wiki/index.php/Transfer_files_from_BO_to_IQ_SFTP_server).
-* Tested with [Node.js v14 LTS](https://nodejs.org/en/).
 
 ## Install
 
-First, install the [Node.js runtime](https://nodejs.org/en/download/).
-
-Next, clone this repository, enter its top-level directory, and install the required NPM modules:
-
-```bash
-cd recfu-mover
-npm install --production && npm audit fix
-```
-
-Next, copy the example configuration:
+1. Install [Node.js v14 LTS](https://nodejs.org/en/download/)
+2. Clone this repo and cd to its top-level directory
+3. Install required packages: `npm ci --production && npm audit fix`
+4. Copy the example configuration:
 
 ```bash
 cp ./site-config/client.json.EXAMPLE ./site-config/client.json
@@ -25,7 +18,6 @@ cp ./site-config/client.json.EXAMPLE ./site-config/client.json
 
 .. and set `client.json` values as needed.
 
-
 ## Run
 
 From this application's top-level directory, run the app using:

app.js → lib/app.js

 'use strict';
 
 // -------------------------------------------------------------------------
-// Variable definitions
+//                  VARIABLE DEFINITIONS
 // -------------------------------------------------------------------------
 
-const {doIt} = require('./lib');
-const options = './site-config/client.json';
+const {downloadFile} = require('./sftp.js');
+const {logError} = require('./errors.js');
+const {readConfig} = require('./config.js');
+const CONFIG_FILE = './site-config/client.json';
 
 // -------------------------------------------------------------------------
-// Main logic
+//                  MAIN LOGIC
 // -------------------------------------------------------------------------
 
-doIt(options, (err, msg) => {
-  if (err) throw err;
-  console.log(msg);
-});
+const conf = readConfig(CONFIG_FILE);
+
+downloadFile(conf)
+  .then(res => console.log(res))
+  .catch(err => logError(err));

lib/config.js

+'use strict';
+
+// -------------------------------------------------------------------------
+//                  VARIABLE DEFINITIONS
+// -------------------------------------------------------------------------
+
+const fs = require('fs');
+
+// -------------------------------------------------------------------------
+//                  FUNCTIONS
+// -------------------------------------------------------------------------
+
+/**
+ * Validate configuration for expected properties.
+ *
+ * @param {string} json - Configuration.
+ * @returns {object} Configuration.
+ */
+function validateConfig(json) {
+  const o = JSON.parse(json);
+  const props = ['sshSettings', 'localFile', 'remoteFile'];
+  props.forEach(x => {
+    if (!o.hasOwnProperty(x)) throw new Error(`Missing ${x} config`);
+  });
+  return o;
+}
+
+/**
+ * Read configuration from JSON file.
+ *
+ * @param {string} file - Name of file.
+ * @returns {object} Configuration.
+ */
+function readConfig(file) {
+  try {
+    const data = fs.readFileSync(file);
+    return validateConfig(data);
+  } catch (err) {
+    console.error(`Problem with config file (${file})`);
+    throw err;
+  }
+}
+
+// -------------------------------------------------------------------------
+//                  EXPORTS
+// -------------------------------------------------------------------------
+
+exports.readConfig = readConfig;

lib/index.spec.js → lib/config.spec.js

@@ -9,28 +9,29 @@
 const assert = require('assert');
 const fs = require('fs');
 const rewire = require('rewire');
-const pf = rewire('../lib');
+const pf = rewire('./config.js');
 
 // Map rewired, private functions to friendlier names
-const readSftpOptions = pf.__get__('readSftpOptions');
+const readConfig = pf.__get__('readConfig');
 
 const liveConf = './site-config/client.json';
-const exampleConf = liveConf + '.EXAMPLE';
+const exampleConf = `${liveConf}.EXAMPLE`;
+const configs = [exampleConf, liveConf];
 
 // --------------------------------------------------------------------------
 //                      MOCHA TESTS
 // --------------------------------------------------------------------------
 
 describe('Filesystem & JSON parsing integration', function() {
-  const configs = [exampleConf];
-  if (fs.existsSync(liveConf)) {
-    configs.push(liveConf);
-  }
-  for (let i = 0; i < configs.length; i++) {
+  for (const config of configs) {
+    if (!fs.existsSync(config)) {
+      continue;
+    }
     let o;
-    describe('Read ' + configs[i], function() {
+    describe('Read ' + config, function() {
       it('should return an object', function() {
-        o = readSftpOptions(configs[i]);
+        o = readConfig(config);
+        assert.equal(typeof o, 'object');
       });
       it('should contain an SSH setting property', function() {
         assert.equal(o.hasOwnProperty('sshSettings'), true);

lib/errors.js

+'use strict';
+
+// ---------------------------------------------------------------------------
+//                  FUNCTIONS
+// ---------------------------------------------------------------------------
+
+/**
+ * Log Error object appropriately.
+ *
+ * @param {object} err - Error object.
+ */
+function logError(err) {
+  if (!err || !(err instanceof Error)) {
+    throw new Error('Unknown problem occurred (contact programmer)');
+  } else {
+    console.error(err.message);
+    console.error(err.stack);
+  }
+}
+
+// ---------------------------------------------------------------------------
+//                  EXPORTS
+// ---------------------------------------------------------------------------
+
+exports.logError = logError;

lib/index.js

-'use strict';
-
-/* eslint consistent-return: 0 */
-/* eslint "jsdoc/require-returns-check": 0 */
-
-// -------------------------------------------------------------------------
-// Variable definitions
-// -------------------------------------------------------------------------
-
-const fs = require('fs');
-const SshClient = require('ssh2').Client;
-const ssh = new SshClient();
-
-// -------------------------------------------------------------------------
-// Helper functions
-// -------------------------------------------------------------------------
-
-/**
- * Read SFTP options from file.
- *
- * @param {string} json - Name of JSON file.
- * @returns {object} SFTP options.
- */
-function readSftpOptions(json) {
-  try {
-    const o = fs.readFileSync(json);
-    return JSON.parse(o);
-  } catch (err) {
-    console.error(`Options read error: ${err}`);
-    throw err;
-  }
-}
-
-/**
- * Download file from SFTP server.
- *
- * @param {object} sshOpt - SFTP options.
- * @param {string} remoteFile - Name of file on remote SFTP server.
- * @param {string} localFile - Name to save file as locally.
- * @param {Function} callback - Callback function.
- * @returns {Function} Callback function.
- */
-function downloadFile(sshOpt, remoteFile, localFile, callback) {
-  ssh.on('ready', () => {
-    ssh.sftp((err, sftp) => {
-      if (err) {
-        console.error('Problem starting SFTP session');
-        return callback(err);
-      }
-      sftp.exists(remoteFile, (doesExist) => {
-        if (!doesExist) {
-          // This will commonly be the case since we are polling for the
-          // presence of a file (i.e. this is not an error). Return with
-          // an informational message and be done.
-          return callback(null, `Remote file (${remoteFile}) not found`);
-        }
-        sftp.fastGet(remoteFile, localFile, (err) => {
-          if (err) {
-            console.error('Problem downloading');
-            return callback(err);
-          }
-          sftp.unlink(remoteFile, (err) => {
-            if (err) {
-              // Don't blow up if we cannot delete the file on the remote
-              // system, but give a warning about it.
-              console.error('Problem removing remote file');
-            }
-            return callback(null, `Downloaded to ${localFile}`);
-          });
-        });
-      });
-    });
-  });
-  ssh.on('error', (err) => {
-    console.error('Problem connecting to SSH server');
-    return callback(err);
-  });
-  ssh.connect(sshOpt);
-}
-
-// -------------------------------------------------------------------------
-// Exported functions
-// -------------------------------------------------------------------------
-
-/**
- * Read SFTP options, download file from SFTP server.
- *
- * @param {string} optionsConf - Name of SFTP options file.
- * @param {Function} callback - Callback function.
- * @returns {Function} Callback function.
- */
-exports.doIt = function(optionsConf, callback) {
-  const o = readSftpOptions(optionsConf);
-  downloadFile(o.sshSettings, o.remoteFile, o.localFile, (err, msg) => {
-    if (err) {
-      ssh.end();
-      return callback(err);
-    }
-    ssh.end();
-    return callback(null, msg);
-  });
-};

lib/sftp.js

+'use strict';
+
+/* eslint consistent-return: 0 */
+/* eslint "jsdoc/require-returns-check": 0 */
+
+// -------------------------------------------------------------------------
+//                  VARIABLE DEFINITIONS
+// -------------------------------------------------------------------------
+
+const SshClient = require('ssh2').Client;
+const ssh = new SshClient();
+
+// -------------------------------------------------------------------------
+//                  FUNCTIONS
+// -------------------------------------------------------------------------
+
+/**
+ * Close SSH connection.
+ *
+ * @param {object} o - SSH instance.
+ * @returns {undefined|Function} Response message.
+ */
+function closeConnection(o) {
+  if (!o || typeof o !== 'object') return;
+  if (typeof o.end !== 'function') return;
+  return o.end();
+}
+
+/**
+ * Download file from SFTP server.
+ *
+ * @param {object} conf - Configuration.
+ * @returns {Promise<string>} Response message.
+ */
+function downloadFile(conf) {
+  return new Promise((resolve, reject) => {
+    const rf = conf.remoteFile;
+    const lf = conf.localFile;
+    ssh.on('ready', () => {
+      ssh.sftp((err, sftp) => {
+        if (err) {
+          closeConnection(ssh);
+          return reject(err);
+        }
+        sftp.exists(rf, (found) => {
+          // No problem if the file does not exist.
+          if (!found) {
+            closeConnection(ssh);
+            return resolve(`Remote file (${rf}) not found`);
+          }
+          sftp.fastGet(rf, lf, (err) => {
+            if (err) {
+              closeConnection(ssh);
+              return reject(err);
+            }
+            sftp.unlink(rf, (err) => {
+              // Warn if we cannot delete the remote file.
+              if (err) {
+                console.error(`Problem deleting remote file (${rf})`);
+              }
+              closeConnection(ssh);
+              return resolve(`Downloaded to ${lf}`);
+            });
+          });
+        });
+      });
+    });
+    ssh.on('error', (err) => {
+      closeConnection(ssh);
+      return reject(err);
+    });
+    ssh.connect(conf.sshSettings);
+  });
+}
+
+// -------------------------------------------------------------------------
+//                  EXPORTS
+// -------------------------------------------------------------------------
+
+exports.downloadFile = downloadFile;

package.json

@@ -14,7 +14,7 @@
   "scripts": {
     "pretest": "eslint --ignore-path .eslintignore .",
     "test": "mocha 'lib/*.spec.js'",
-    "download": "node app.js"
+    "download": "node lib/app.js"
   },
   "author": "whatitis",
   "license": "MIT",