From 01dc7dc1d8b7b4571cee645884ae88372545acbd Mon Sep 17 00:00:00 2001
From: Juan Picado <juanpicado19@gmail.com>
Date: Sat, 20 May 2017 08:56:06 +0200
Subject: [PATCH] Refactor, reduce eslint warnings, add doc

---
 lib/cli.js           |  40 ++++----
 lib/config-path.js   |  15 +--
 lib/local-storage.js | 217 ++++++++++++++++++++++++++-----------------
 lib/middleware.js    |  24 +++--
 4 files changed, 177 insertions(+), 119 deletions(-)

diff --git a/lib/cli.js b/lib/cli.js
index 63dbfed5e..1c9af8fa9 100644
--- a/lib/cli.js
+++ b/lib/cli.js
@@ -1,6 +1,7 @@
 #!/usr/bin/env node
 
-/* eslint no-sync:0*/
+/* eslint no-sync:0 */
+/* eslint no-empty:0 */
 'use strict';
 
 if (process.getuid && process.getuid() === 0) {
@@ -13,24 +14,24 @@ try {
   // for debugging memory leaks
   // totally optional
   require('heapdump');
-} catch(err) {}
+} catch(err) { }
 
-let logger = require('./logger');
+const logger = require('./logger');
 logger.setup(); // default setup
 
-let commander = require('commander');
-let constants = require('constants');
-let fs = require('fs');
-let http = require('http');
-let https = require('https');
-let YAML = require('js-yaml');
-let Path = require('path');
-let URL = require('url');
-let server = require('./index');
-let Utils = require('./utils');
-let pkginfo = require('pkginfo')(module); // eslint-disable-line no-unused-vars
-let pkgVersion = module.exports.version;
-let pkgName = module.exports.name;
+const commander = require('commander');
+const constants = require('constants');
+const fs = require('fs');
+const http = require('http');
+const https = require('https');
+const YAML = require('js-yaml');
+const Path = require('path');
+const URL = require('url');
+const server = require('./index');
+const Utils = require('./utils');
+const pkginfo = require('pkginfo')(module); // eslint-disable-line no-unused-vars
+const pkgVersion = module.exports.version;
+const pkgName = module.exports.name;
 
 commander
   .option('-l, --listen <[host:]port>', 'host:port number to listen on (default: localhost:4873)')
@@ -71,6 +72,7 @@ afterConfigLoad();
  *  listen:
     - localhost:5555
     - localhost:5557
+    @return {Array}
  */
 function get_listen_addresses() {
   // command line || config file || default
@@ -113,7 +115,6 @@ function afterConfigLoad() {
   const app = server(config);
   get_listen_addresses().forEach(function(addr) {
     let webServer;
-
     if (addr.proto === 'https') { // https
       if (!config.https || !config.https.key || !config.https.cert || !config.https.ca) {
         let conf_path = function(file) {
@@ -128,7 +129,8 @@ function afterConfigLoad() {
           'To quickly create self-signed certificate, use:',
           ' $ openssl genrsa -out ' + conf_path('verdaccio-key.pem') + ' 2048',
           ' $ openssl req -new -sha256 -key ' + conf_path('verdaccio-key.pem') + ' -out ' + conf_path('verdaccio-csr.pem'),
-          ' $ openssl x509 -req -in ' + conf_path('verdaccio-csr.pem') + ' -signkey ' + conf_path('verdaccio-key.pem') + ' -out ' + conf_path('verdaccio-cert.pem'),
+          ' $ openssl x509 -req -in ' + conf_path('verdaccio-csr.pem') +
+          ' -signkey ' + conf_path('verdaccio-key.pem') + ' -out ' + conf_path('verdaccio-cert.pem'),
           '',
           'And then add to config file (' + conf_path() + '):',
           '  https:',
@@ -186,7 +188,7 @@ function afterConfigLoad() {
 }
 
 process.on('uncaughtException', function(err) {
-  logger.logger.fatal( {err: err}, 
+  logger.logger.fatal( {err: err},
   'uncaught exception, please report this\n@{err.stack}' );
   process.exit(255);
 });
diff --git a/lib/config-path.js b/lib/config-path.js
index b83aa0e72..6ddfa2dbc 100644
--- a/lib/config-path.js
+++ b/lib/config-path.js
@@ -1,13 +1,15 @@
 'use strict';
 
-let fs = require('fs');
-let Path = require('path');
-let logger = require('./logger');
-
-module.exports = find_config_file;
+const fs = require('fs');
+const Path = require('path');
+const logger = require('./logger');
 
+/**
+ * 
+ * @return {String} the config file path
+ */
 function find_config_file() {
-  let paths = get_paths();
+  const paths = get_paths();
 
   for (let i=0; i<paths.length; i++) {
     if (file_exists(paths[i].path)) return paths[i].path;
@@ -89,3 +91,4 @@ function file_exists(path) {
   return stat.isFile();
 }
 
+module.exports = find_config_file;
diff --git a/lib/local-storage.js b/lib/local-storage.js
index f85746195..3c5ca0d63 100644
--- a/lib/local-storage.js
+++ b/lib/local-storage.js
@@ -1,3 +1,5 @@
+/* eslint prefer-rest-params: "off" */
+/* eslint prefer-spread: "off" */
 'use strict';
 
 const assert = require('assert');
@@ -16,7 +18,7 @@ const Utils = require('./utils');
 const info_file = 'package.json';
 
 // returns the minimal package file
-function get_boilerplate(name) {
+const get_boilerplate = function(name) {
   return {
     // standard things
     'name': name,
@@ -28,35 +30,40 @@ function get_boilerplate(name) {
     '_attachments': {},
     '_uplinks': {},
   };
-}
+};
 
-//
-// Implements Storage interface
-// (same for storage.js, local-storage.js, up-storage.js)
-//
+/**
+ * Implements Storage interface (same for storage.js, local-storage.js, up-storage.js).
+ */
 class Storage {
+  /**
+   * Constructor
+   * @param {Object} config config list of properties
+   */
   constructor(config) {
     this.config = config;
     this.logger = Logger.logger.child({sub: 'fs'});
   }
 
   /**
-   *
+   * Handle internal error
    * @param {*} err
    * @param {*} file
    * @param {*} message
+   * @return {Object} Error instance
    */
   _internal_error(err, file, message) {
-    this.logger.error( {err: err, file: file}
-                    , message + ' @{file}: @{!err.message}' );
+    this.logger.error( {err: err, file: file},
+    message + ' @{file}: @{!err.message}' );
     return Error[500]();
   }
 
   /**
-   *
+   * Add a package.
    * @param {*} name
    * @param {*} info
    * @param {*} callback
+   * @return {Function}
    */
   add_package(name, info, callback) {
     let storage = this.storage(name);
@@ -76,16 +83,19 @@ class Storage {
   }
 
   /**
-   *
+   * Remove package.
    * @param {*} name
    * @param {*} callback
+   * @return {Function}
    */
   remove_package(name, callback) {
     this.logger.info( {name: name}
                     , 'unpublishing @{name} (all)');
 
     let storage = this.storage(name);
-    if (!storage) return callback( Error[404]('no such package available') );
+    if (!storage) {
+      return callback( Error[404]('no such package available') );
+    }
 
     storage.read_json(info_file, (err, data) => {
       if (err) {
@@ -98,18 +108,22 @@ class Storage {
       this._normalize_package(data);
 
       storage.unlink(info_file, function(err) {
-        if (err) return callback(err);
+        if (err) {
+          return callback(err);
+        }
 
-        let files = Object.keys(data._attachments);
+        const files = Object.keys(data._attachments);
 
-        function unlinkNext(cb) {
-          if (files.length === 0) return cb();
+        const unlinkNext = function(cb) {
+          if (files.length === 0) {
+            return cb();
+          }
 
           let file = files.shift();
           storage.unlink(file, function() {
             unlinkNext(cb);
           });
-        }
+        };
 
         unlinkNext(function() {
           // try to unlink the directory, but ignore errors because it can fail
@@ -125,12 +139,13 @@ class Storage {
   }
 
   /**
-   *
+   * Retrieve either a previous created local package or a boilerplate.
    * @param {*} name
    * @param {*} callback
+   * @return {Function}
    */
   _read_create_package(name, callback) {
-    let storage = this.storage(name);
+    const storage = this.storage(name);
     if (!storage) {
       let data = get_boilerplate(name);
       this._normalize_package(data);
@@ -159,7 +174,9 @@ class Storage {
    */
   update_versions(name, newdata, callback) {
     this._read_create_package(name, (err, data) => {
-      if (err) return callback(err);
+      if (err) {
+        return callback(err);
+      }
 
       let change = false;
       for (let ver in newdata.versions) {
@@ -206,13 +223,15 @@ class Storage {
         }
       }
       for (let up in newdata._uplinks) {
-        let need_change = !Utils.is_object(data._uplinks[up])
-                      || newdata._uplinks[up].etag !== data._uplinks[up].etag
-                      || newdata._uplinks[up].fetched !== data._uplinks[up].fetched;
+        if (Object.prototype.hasOwnProperty.call(newdata._uplinks, up)) {
+          const need_change = !Utils.is_object(data._uplinks[up])
+                        || newdata._uplinks[up].etag !== data._uplinks[up].etag
+                        || newdata._uplinks[up].fetched !== data._uplinks[up].fetched;
 
-        if (need_change) {
-          change = true;
-          data._uplinks[up] = newdata._uplinks[up];
+          if (need_change) {
+            change = true;
+            data._uplinks[up] = newdata._uplinks[up];
+          }
         }
       }
       if (newdata.readme !== data.readme) {
@@ -232,7 +251,7 @@ class Storage {
   }
 
   /**
-   *
+   * Add a new version to a previous local package.
    * @param {*} name
    * @param {*} version
    * @param {*} metadata
@@ -272,7 +291,7 @@ class Storage {
   }
 
   /**
-   *
+   * Merge a new list of tags for a local packages with the existing one.
    * @param {*} name
    * @param {*} tags
    * @param {*} callback
@@ -284,7 +303,7 @@ class Storage {
           delete data['dist-tags'][t];
           continue;
         }
-
+        // be careful here with == (cast)
         if (data.versions[tags[t]] == null) {
           return cb( Error[404]('this version doesn\'t exist') );
         }
@@ -295,8 +314,8 @@ class Storage {
     }, callback);
   }
 
-  /**
-   *
+ /**
+   * Replace the complete list of tags for a local package.
    * @param {*} name
    * @param {*} tags
    * @param {*} callback
@@ -322,11 +341,13 @@ class Storage {
   }
 
   /**
-   * Currently supports unpublishing only
+   * Update the package metadata, tags and attachments (tarballs).
+   * Note: Currently supports unpublishing only.
    * @param {*} name
    * @param {*} metadata
    * @param {*} revision
    * @param {*} callback
+   * @return {Function}
    */
   change_package(name, metadata, revision, callback) {
     if (!Utils.is_object(metadata.versions) || !Utils.is_object(metadata['dist-tags'])) {
@@ -336,10 +357,9 @@ class Storage {
     this.update_package(name, (data, cb) => {
       for (let ver in data.versions) {
         if (metadata.versions[ver] == null) {
-          this.logger.info( {name: name, version: ver}
-                          , 'unpublishing @{name}@@{version}');
+          this.logger.info( {name: name, version: ver},
+          'unpublishing @{name}@@{version}');
           delete data.versions[ver];
-
           for (let file in data._attachments) {
             if (data._attachments[file].version === ver) {
               delete data._attachments[file].version;
@@ -350,13 +370,15 @@ class Storage {
       data['dist-tags'] = metadata['dist-tags'];
       cb();
     }, function(err) {
-      if (err) return callback(err);
+      if (err) {
+        return callback(err);
+      }
       callback();
     });
   }
 
   /**
-   *
+   * Remove a tarball.
    * @param {*} name
    * @param {*} filename
    * @param {*} revision
@@ -384,9 +406,10 @@ class Storage {
   }
 
   /**
-   *
-   * @param {*} name
-   * @param {*} filename
+   * Add a tarball.
+   * @param {String} name
+   * @param {String} filename
+   * @return {Function}
    */
   add_tarball(name, filename) {
     assert(Utils.validate_name(filename));
@@ -473,28 +496,29 @@ class Storage {
   }
 
   /**
-   *
+   * Get a tarball.
    * @param {*} name
    * @param {*} filename
    * @param {*} callback
+   * @return {Function}
    */
   get_tarball(name, filename, callback) {
     assert(Utils.validate_name(filename));
-    let self = this;
-
-    let stream = MyStreams.readTarballStream();
+    const stream = MyStreams.readTarballStream();
     stream.abort = function() {
-      if (rstream) rstream.abort();
+      if (rstream) {
+        rstream.abort();
+      }
     };
 
-    let storage = self.storage(name);
+    let storage = this.storage(name);
     if (!storage) {
       process.nextTick(function() {
         stream.emit('error', Error[404]('no such file available'));
       });
       return stream;
     }
-
+    /* eslint no-var: "off" */
     var rstream = storage.read_stream(filename);
     rstream.on('error', function(err) {
       if (err && err.code === 'ENOENT') {
@@ -515,10 +539,11 @@ class Storage {
   }
 
   /**
-   *
+   * Retrieve a package by name.
    * @param {*} name
    * @param {*} options
    * @param {*} callback
+   * @return {Function}
    */
   get_package(name, options, callback) {
     if (typeof(options) === 'function') {
@@ -526,7 +551,9 @@ class Storage {
     }
 
     let storage = this.storage(name);
-    if (!storage) return callback( Error[404]('no such package available') );
+    if (!storage) {
+      return callback( Error[404]('no such package available') );
+    }
 
     storage.read_json(info_file, (err, result) => {
       if (err) {
@@ -542,7 +569,7 @@ class Storage {
   }
 
   /**
-   * Walks through each package and calls `on_package` on them
+   * Walks through each package and calls `on_package` on them.
    * @param {*} on_package
    * @param {*} on_end
    */
@@ -550,7 +577,6 @@ class Storage {
     let storages = {};
 
     storages[this.config.storage] = true;
-
     if (this.config.packages) {
       Object.keys(this.packages || {}).map( (pkg) => {
         if (this.config.packages[pkg].storage) {
@@ -562,13 +588,17 @@ class Storage {
 
     async.eachSeries(Object.keys(storages), function(storage, cb) {
       fs.readdir(Path.resolve(base, storage), function(err, files) {
-        if (err) return cb(err);
+        if (err) {
+          return cb(err);
+        }
 
           async.eachSeries(files, function(file, cb) {
             if (file.match(/^@/)) {
               // scoped
               fs.readdir(Path.resolve(base, storage, file), function(err, files) {
-                if (err) return cb(err);
+                if (err) {
+                  return cb(err);
+                }
 
                 async.eachSeries(files, function(file2, cb) {
                   if (Utils.validate_name(file2)) {
@@ -606,16 +636,18 @@ class Storage {
      * @param {*} name package name
      * @param {*} updateFn function(package, cb) - update function
      * @param {*} _callback callback that gets invoked after it's all updated
+     * @return {Function}
      */
     update_package(name, updateFn, _callback) {
-      let self = this;
-      let storage = self.storage(name);
-      if (!storage) return _callback( Error[404]('no such package available') );
-      storage.lock_and_read_json(info_file, function(err, json) {
+      const storage = this.storage(name);
+      if (!storage) {
+        return _callback( Error[404]('no such package available') );
+      }
+      storage.lock_and_read_json(info_file, (err, json) => {
         let locked = false;
 
         // callback that cleans up lock first
-        function callback(err) {
+        const callback = function(err) {
           let _args = arguments;
           if (locked) {
             storage.unlock_file(info_file, function() {
@@ -625,7 +657,7 @@ class Storage {
           } else {
             _callback.apply(null, _args);
           }
-        }
+        };
 
         if (!err) {
           locked = true;
@@ -641,19 +673,21 @@ class Storage {
           }
         }
 
-        self._normalize_package(json);
-        updateFn(json, function(err) {
-          if (err) return callback(err);
-
-          self._write_package(name, json, callback);
+        this._normalize_package(json);
+        updateFn(json, (err) => {
+          if (err) {
+            return callback(err);
+          }
+          this._write_package(name, json, callback);
         });
       });
     }
 
     /**
-     *
+     * Search a local package.
      * @param {*} startkey
      * @param {*} options
+     * @return {Function}
      */
     search(startkey, options) {
       const stream = new Stream.PassThrough({objectMode: true});
@@ -709,12 +743,14 @@ class Storage {
     }
 
     /**
-     *
-     * @param {*} pkg
+     * Normalise package properties, tags, revision id.
+     * @param {Object} pkg package reference.
      */
     _normalize_package(pkg) {
       ['versions', 'dist-tags', '_distfiles', '_attachments', '_uplinks'].forEach(function(key) {
-        if (!Utils.is_object(pkg[key])) pkg[key] = {};
+        if (!Utils.is_object(pkg[key])) {
+          pkg[key] = {};
+        }
       });
       if (typeof(pkg._rev) !== 'string') {
         pkg._rev = '0-0000000000000000';
@@ -724,10 +760,11 @@ class Storage {
     }
 
     /**
-     *
+     * Update the revision (_rev) string for a package.
      * @param {*} name
      * @param {*} json
      * @param {*} callback
+     * @return {Function}
      */
     _write_package(name, json, callback) {
       // calculate revision a la couchdb
@@ -738,13 +775,16 @@ class Storage {
       json._rev = ((+rev[0] || 0) + 1) + '-' + Crypto.pseudoRandomBytes(8).toString('hex');
 
       let storage = this.storage(name);
-      if (!storage) return callback();
+      if (!storage) {
+        return callback();
+      }
       storage.write_json(info_file, json, callback);
     }
 
     /**
-     *
-     * @param {*} pkg
+     * Retrieve a wrapper that provide access to the package location.
+     * @param {*} pkg package name.
+     * @return {Object}
      */
     storage(pkg) {
       let path = this.config.get_package_spec(pkg).storage;
@@ -756,7 +796,7 @@ class Storage {
                         , 'this package has no storage defined: @{name}' );
         return null;
       }
-      return Path_Wrapper(
+      return new PathWrapper(
         Path.join(
           Path.resolve(Path.dirname(this.config.self_path || ''), path),
           pkg
@@ -765,26 +805,33 @@ class Storage {
     }
 }
 
-var Path_Wrapper = (function() {
-  // a wrapper adding paths to fs_storage methods
-  function Wrapper(path) {
-    let self = Object.create(Wrapper.prototype);
-    self.path = path;
-    return self;
-  }
+const PathWrapper = (function() {
+  /**
+   * A wrapper adding paths to fs_storage methods.
+   */
+  class Wrapper {
 
-  for (let i in fs_storage) {
-    if (fs_storage.hasOwnProperty(i)) {
-      Wrapper.prototype[i] = wrapper(i);
+    /**
+     * @param {*} path
+     */
+    constructor(path) {
+      this.path = path;
     }
   }
 
-  function wrapper(method) {
-    return function(/* ...*/) {
+  const wrapLocalStorageMethods = function(method) {
+    return function() {
       let args = Array.prototype.slice.apply(arguments);
+      /* eslint no-invalid-this: off */
       args[0] = Path.join(this.path, args[0] || '');
       return fs_storage[method].apply(null, args);
     };
+  };
+
+  for (let i in fs_storage) {
+    if (fs_storage.hasOwnProperty(i)) {
+      Wrapper.prototype[i] = wrapLocalStorageMethods(i);
+    }
   }
 
   return Wrapper;
diff --git a/lib/middleware.js b/lib/middleware.js
index 38f0a3963..988af279e 100644
--- a/lib/middleware.js
+++ b/lib/middleware.js
@@ -1,9 +1,11 @@
+/* eslint prefer-rest-params: "off" */
+
 'use strict';
 
-let crypto = require('crypto');
-let Error = require('http-errors');
-let utils = require('./utils');
-let Logger = require('./logger');
+const crypto = require('crypto');
+const Error = require('http-errors');
+const utils = require('./utils');
+const Logger = require('./logger');
 
 module.exports.match = function match(regexp) {
   return function(req, res, next, value, name) {
@@ -71,9 +73,13 @@ module.exports.anti_loop = function(config) {
   };
 };
 
-// express doesn't do etags with requests <= 1024b
-// we use md5 here, it works well on 1k+ bytes, but sucks with fewer data
-// could improve performance using crc32 after benchmarks
+/**
+ * Express doesn't do etags with requests <= 1024b
+ * we use md5 here, it works well on 1k+ bytes, but sucks with fewer data
+ * could improve performance using crc32 after benchmarks.
+ * @param {Object} data
+ * @return {String}
+ */
 function md5sum(data) {
   return crypto.createHash('md5').update(data).digest('hex');
 }
@@ -168,7 +174,7 @@ module.exports.log = function(req, res, next) {
     _write.apply(res, arguments);
   };
 
-  function log() {
+  const log = function() {
     let message = '@{status}, user: @{user}, req: \'@{request.method} @{request.url}\'';
     if (res._verdaccio_error) {
       message += ', error: @{!error}';
@@ -189,7 +195,7 @@ module.exports.log = function(req, res, next) {
       },
     }, message);
     req.originalUrl = req.url;
-  }
+  };
 
   req.on('close', function() {
     log(true);