Refactor storage, moves secret key generator to config file

lib/config.js

@@ -4,11 +4,10 @@
 'use strict';
 
 const assert = require('assert');
-const Crypto = require('crypto');
+const _ = require('lodash');
 const Error = require('http-errors');
+const Crypto = require('crypto');
 const minimatch = require('minimatch');
-const Path = require('path');
-const LocalData = require('./local-data');
 const Utils = require('./utils');
 const pkginfo = require('pkginfo')(module); // eslint-disable-line no-unused-vars
 const pkgVersion = module.exports.version;
@@ -93,24 +92,6 @@ class Config {
     assert.equal(typeof(config), 'object', 'CONFIG: it doesn\'t look like a valid config file');
 
     assert(self.storage, 'CONFIG: storage path not defined');
-    // local data handler is linked with the configuration handler
-    self.localList = new LocalData(
-      Path.join(
-        Path.resolve(Path.dirname(self.self_path || ''), self.storage),
-        // FUTURE: the database might be parameterizable from config.yaml
-        '.sinopia-db.json'
-      )
-    );
-    // it generates a secret key
-    // FUTURE: this might be an external secret key, perhaps whitin config file?
-    if (!self.secret) {
-      self.secret = self.localList.data.secret;
-      if (!self.secret) {
-        self.secret = Crypto.pseudoRandomBytes(32).toString('hex');
-        self.localList.data.secret = self.secret;
-        self.localList.sync();
-      }
-    }
 
     const users = {
       'all': true,
@@ -257,6 +238,21 @@ class Config {
     }
     return {};
   }
+
+  /**
+   * Store or create whether recieve a secret key
+   * @param {String} secret
+   * @return {String}
+   */
+  checkSecretKey(secret) {
+    if (_.isNil(secret) === false) {
+      return secret;
+    }
+    // it generates a secret key
+    // FUTURE: this might be an external secret key, perhaps whitin config file?
+    this.secret = Crypto.pseudoRandomBytes(32).toString('hex');
+    return this.secret;
+  }
 }
 
 module.exports = Config;

lib/storage.js

@@ -1,12 +1,12 @@
 'use strict';
 
+const _ = require('lodash');
 const assert = require('assert');
 const async = require('async');
 const Error = require('http-errors');
 const semver = require('semver');
-const Crypto = require('crypto');
-const _ = require('lodash');
 const Stream = require('stream');
+
 const Search = require('./search');
 const LocalStorage = require('./local-storage');
 const Logger = require('./logger');
@@ -14,7 +14,6 @@ const MyStreams = require('./streams');
 const Proxy = require('./up-storage');
 const Utils = require('./utils');
 
-
 /**
  * Implements Storage interface
  * (same for storage.js, local-storage.js, up-storage.js).
@@ -22,32 +21,14 @@ const Utils = require('./utils');
 class Storage {
 
   /**
-   *
    * @param {*} config
    */
   constructor(config) {
     this.config = config;
-    // we support a number of uplinks, but only one local storage
+    this._setupUpLinks(this.config);
-    // Proxy and Local classes should have similar API interfaces
-    this.uplinks = {};
-    for (let p in config.uplinks) {
-      if (Object.prototype.hasOwnProperty.call(config.uplinks, p)) {
-        // instance for each up-link definition
-        this.uplinks[p] = new Proxy(config.uplinks[p], config);
-        this.uplinks[p].upname = p;
-      }
-    }
     this.localStorage = new LocalStorage(config);
-    // it generates a secret key
+    this.localStorage.localList.data.secret = this.config.checkSecretKey(this.localStorage.localList.data.secret);
-    // FUTURE: this might be an external secret key, perhaps whitin config file?
-    if (!config.secret) {
-      config.secret = this.localStorage.localList.data.secret;
-      if (!config.secret) {
-        config.secret = Crypto.pseudoRandomBytes(32).toString('hex');
-        this.localStorage.localList.data.secret = config.secret;
     this.localStorage.localList.sync();
-      }
-    }
     // an instance for local storage
     this.logger = Logger.logger.child();
   }
@@ -66,7 +47,6 @@ class Storage {
 
     /**
      * Check whether a package it is already a local package
-     * @param {*} cb the callback method
      * @return {Promise}
      */
     const checkPackageLocal = () => {
@@ -85,7 +65,6 @@ class Storage {
 
     /**
      * Check whether a package exist in any of the uplinks.
-     * @param {*} cb the callback method
      * @return {Promise}
      */
     const check_package_remote = () => {
@@ -102,7 +81,7 @@ class Storage {
           for (let i = 0; i < err_results.length; i++) {
             // checking error
             // if uplink fails with a status other than 404, we report failure
-            if (err_results[i][0] != null) {
+            if (_.isNil(err_results[i][0]) === false) {
               if (err_results[i][0].status !== 404) {
                 return reject(Error[503]('one of the uplinks is down, refuse to publish'));
               }
@@ -116,7 +95,6 @@ class Storage {
 
     /**
      * Add a package to the local database
-     * @param {*} cb callback method
      * @return {Promise}
      */
     const publish_package = () => {
@@ -212,7 +190,7 @@ class Storage {
     Function removes a tarball from local storage.
     Tarball in question should not be linked to in any existing
     versions, i.e. package version should be unpublished first.
-    Used storages: local (write)
+    Used storage: local (write)
    * @param {*} name
    * @param {*} filename
    * @param {*} revision
@@ -266,7 +244,7 @@ class Storage {
       rstream.abort();
       rstream = null; // gc
       self.localStorage.getPackage(name, function(err, info) {
-        if (!err && info._distfiles && info._distfiles[filename] != null) {
+        if (!err && info._distfiles && _.isNil(info._distfiles[filename]) === false) {
           // information about this file exists locally
           serve_file(info._distfiles[filename]);
         } else {
@@ -275,7 +253,7 @@ class Storage {
             if (err) {
                return readStream.emit('error', err);
             }
-            if (!info._distfiles || info._distfiles[filename] == null) {
+            if (!info._distfiles || _.isNil(info._distfiles[filename])) {
               return readStream.emit('error', err404);
             }
             serve_file(info._distfiles[filename]);
@@ -604,6 +582,24 @@ class Storage {
     });
   }
 
+  /**
+   * Set up the Up Storage for each link.
+   * @param {Object} config
+   * @private
+   */
+  _setupUpLinks(config) {
+    // we support a number of uplinks, but only one local storage
+    // Proxy and Local classes should have similar API interfaces
+    this.uplinks = {};
+    for (let p in config.uplinks) {
+      if (Object.prototype.hasOwnProperty.call(config.uplinks, p)) {
+        // instance for each up-link definition
+        this.uplinks[p] = new Proxy(config.uplinks[p], config);
+        this.uplinks[p].upname = p;
+      }
+    }
+  }
+
   /**
    * Function gets a local info and an info from uplinks and tries to merge it
      exported for unit tests only.
@@ -616,7 +612,7 @@ class Storage {
     // copy new versions to a cache
     // NOTE: if a certain version was updated, we can't refresh it reliably
     for (let i in up.versions) {
-      if (local.versions[i] == null) {
+      if (_.isNil(local.versions[i])) {
         local.versions[i] = up.versions[i];
       }
     }