Renamed config index to loader

- Renamed the file in line with our rules around index.js files
- Cleaned up some outdated code patterns
- Want to make the config module a little clearer in what it does

core/shared/config/index.js

@@ -1,92 +1,3 @@
-const Nconf = require('nconf');
+const loader = require('./loader');
-const _ = require('lodash');
-const os = require('os');
-const path = require('path');
-const _debug = require('@tryghost/debug')._base;
-const debug = _debug('ghost:config');
-const localUtils = require('./utils');
-const env = process.env.NODE_ENV || 'development';
-const _private = {};
 
-_private.loadNconf = function loadNconf(options) {
+module.exports = loader.loadNconf();
-    debug('config start');
-    options = options || {};
-
-    const baseConfigPath = options.baseConfigPath || __dirname;
-    const customConfigPath = options.customConfigPath || process.cwd();
-    const nconf = new Nconf.Provider();
-
-    // ## Load Config
-
-    // no channel can override the overrides
-    nconf.file('overrides', path.join(baseConfigPath, 'overrides.json'));
-
-    // command line arguments take precedence, then environment variables
-    nconf.argv();
-    nconf.env({separator: '__', parseValues: true});
-
-    // Now load various config json files
-    nconf.file('custom-env', path.join(customConfigPath, 'config.' + env + '.json'));
-    if (env !== 'testing') {
-        nconf.file('local-env', path.join(customConfigPath, 'config.local.json'));
-    }
-    nconf.file('default-env', path.join(baseConfigPath, 'env', 'config.' + env + '.json'));
-
-    // Finally, we load defaults, if nothing else has a value this will
-    nconf.file('defaults', path.join(baseConfigPath, 'defaults.json'));
-
-    // ## Config Methods
-
-    // Bind internal-only methods, not sure this is needed
-    nconf.makePathsAbsolute = localUtils.makePathsAbsolute.bind(nconf);
-    nconf.sanitizeDatabaseProperties = localUtils.sanitizeDatabaseProperties.bind(nconf);
-    nconf.doesContentPathExist = localUtils.doesContentPathExist.bind(nconf);
-    nconf.checkUrlProtocol = localUtils.checkUrlProtocol.bind(nconf);
-
-    // Expose dynamic utility methods
-    nconf.isPrivacyDisabled = localUtils.isPrivacyDisabled.bind(nconf);
-    nconf.getContentPath = localUtils.getContentPath.bind(nconf);
-
-    // ## Sanitization
-
-    // transform all relative paths to absolute paths
-    nconf.makePathsAbsolute(nconf.get('paths'), 'paths');
-
-    // transform sqlite filename path for Ghost-CLI
-    nconf.sanitizeDatabaseProperties();
-
-    if (nconf.get('database:client') === 'sqlite3') {
-        nconf.makePathsAbsolute(nconf.get('database:connection'), 'database:connection');
-
-        // In the default SQLite test config we set the path to /tmp/ghost-test.db,
-        // but this won't work on Windows, so we need to replace the /tmp bit with
-        // the Windows temp folder
-        const filename = nconf.get('database:connection:filename');
-        if (_.isString(filename) && filename.match(/^\/tmp/)) {
-            nconf.set('database:connection:filename', filename.replace(/^\/tmp/, os.tmpdir()));
-        }
-    }
-
-    // Check if the URL in config has a protocol
-    nconf.checkUrlProtocol();
-
-    // Ensure that the content path exists
-    nconf.doesContentPathExist();
-
-    // ## Other Stuff!
-
-    // Manually set values
-    nconf.set('env', env);
-
-    // Wrap this in a check, because else nconf.get() is executed unnecessarily
-    // To output this, use DEBUG=ghost:*,ghost-config
-    if (_debug.enabled('ghost-config')) {
-        debug(nconf.get());
-    }
-
-    debug('config end');
-    return nconf;
-};
-
-module.exports = _private.loadNconf();
-module.exports.loadNconf = _private.loadNconf;

core/shared/config/loader.js

@@ -0,0 +1,90 @@
+const Nconf = require('nconf');
+const _ = require('lodash');
+const os = require('os');
+const path = require('path');
+const _debug = require('@tryghost/debug')._base;
+const debug = _debug('ghost:config');
+const localUtils = require('./utils');
+const env = process.env.NODE_ENV || 'development';
+
+function loadNconf(options) {
+    debug('config start');
+    options = options || {};
+
+    const baseConfigPath = options.baseConfigPath || __dirname;
+    const customConfigPath = options.customConfigPath || process.cwd();
+    const nconf = new Nconf.Provider();
+
+    // ## Load Config
+
+    // no channel can override the overrides
+    nconf.file('overrides', path.join(baseConfigPath, 'overrides.json'));
+
+    // command line arguments take precedence, then environment variables
+    nconf.argv();
+    nconf.env({separator: '__', parseValues: true});
+
+    // Now load various config json files
+    nconf.file('custom-env', path.join(customConfigPath, 'config.' + env + '.json'));
+    if (env !== 'testing') {
+        nconf.file('local-env', path.join(customConfigPath, 'config.local.json'));
+    }
+    nconf.file('default-env', path.join(baseConfigPath, 'env', 'config.' + env + '.json'));
+
+    // Finally, we load defaults, if nothing else has a value this will
+    nconf.file('defaults', path.join(baseConfigPath, 'defaults.json'));
+
+    // ## Config Methods
+
+    // Bind internal-only methods, not sure this is needed
+    nconf.makePathsAbsolute = localUtils.makePathsAbsolute.bind(nconf);
+    nconf.sanitizeDatabaseProperties = localUtils.sanitizeDatabaseProperties.bind(nconf);
+    nconf.doesContentPathExist = localUtils.doesContentPathExist.bind(nconf);
+    nconf.checkUrlProtocol = localUtils.checkUrlProtocol.bind(nconf);
+
+    // Expose dynamic utility methods
+    nconf.isPrivacyDisabled = localUtils.isPrivacyDisabled.bind(nconf);
+    nconf.getContentPath = localUtils.getContentPath.bind(nconf);
+
+    // ## Sanitization
+
+    // transform all relative paths to absolute paths
+    nconf.makePathsAbsolute(nconf.get('paths'), 'paths');
+
+    // transform sqlite filename path for Ghost-CLI
+    nconf.sanitizeDatabaseProperties();
+
+    if (nconf.get('database:client') === 'sqlite3') {
+        nconf.makePathsAbsolute(nconf.get('database:connection'), 'database:connection');
+
+        // In the default SQLite test config we set the path to /tmp/ghost-test.db,
+        // but this won't work on Windows, so we need to replace the /tmp bit with
+        // the Windows temp folder
+        const filename = nconf.get('database:connection:filename');
+        if (_.isString(filename) && filename.match(/^\/tmp/)) {
+            nconf.set('database:connection:filename', filename.replace(/^\/tmp/, os.tmpdir()));
+        }
+    }
+
+    // Check if the URL in config has a protocol
+    nconf.checkUrlProtocol();
+
+    // Ensure that the content path exists
+    nconf.doesContentPathExist();
+
+    // ## Other Stuff!
+
+    // Manually set values
+    nconf.set('env', env);
+
+    // Wrap this in a check, because else nconf.get() is executed unnecessarily
+    // To output this, use DEBUG=ghost:*,ghost-config
+    if (_debug.enabled('ghost-config')) {
+        debug(nconf.get());
+    }
+
+    debug('config end');
+    return nconf;
+}
+
+module.exports.loadNconf = loadNconf;

test/unit/shared/config/adapter_config_spec.js

@@ -0,0 +1,70 @@
+const should = require('should');
+const path = require('path');
+const configUtils = require('../../../utils/configUtils');
+
+/**
+ * This is a somewhat legacy set of tests that check we get the right values for storage
+ * We should rethink what the purpose of these tests is.
+ */
+
+describe('Adapter Config', function () {
+    before(function () {
+        configUtils.restore();
+    });
+
+    afterEach(function () {
+        configUtils.restore();
+    });
+
+    describe('Storage', function () {
+        it('should default to local-file-store', function () {
+            configUtils.config.get('paths').should.have.property('internalAdaptersPath', path.join(configUtils.config.get('paths').corePath, '/server/adapters/'));
+
+            configUtils.config.get('storage').should.have.property('active', 'LocalFileStorage');
+        });
+
+        it('no effect: setting a custom active storage as string', function () {
+            configUtils.set({
+                storage: {
+                    active: 's3',
+                    s3: {}
+                }
+            });
+
+            configUtils.config.get('storage').should.have.property('active', 's3');
+            configUtils.config.get('storage').should.have.property('s3', {});
+        });
+
+        it('able to set storage for themes (but not officially supported!)', function () {
+            configUtils.set({
+                storage: {
+                    active: {
+                        images: 'local-file-store',
+                        themes: 's3'
+                    }
+                }
+            });
+
+            configUtils.config.get('storage').should.have.property('active', {
+                images: 'local-file-store',
+                themes: 's3'
+            });
+        });
+
+        it('should allow setting a custom active storage as object', function () {
+            configUtils.set({
+                storage: {
+                    active: {
+                        images: 's2',
+                        themes: 'local-file-store'
+                    }
+                }
+            });
+
+            configUtils.config.get('storage').should.have.property('active', {
+                images: 's2',
+                themes: 'local-file-store'
+            });
+        });
+    });
+});

test/unit/shared/config/loader_spec.js

@@ -4,7 +4,7 @@ const rewire = require('rewire');
 const _ = require('lodash');
 const configUtils = require('../../../utils/configUtils');
 
-describe('Config', function () {
+describe('Config Loader', function () {
     before(function () {
         configUtils.restore();
     });
@@ -17,12 +17,12 @@ describe('Config', function () {
         let originalEnv;
         let originalArgv;
         let customConfig;
-        let config;
+        let loader;
 
         beforeEach(function () {
             originalEnv = _.clone(process.env);
             originalArgv = _.clone(process.argv);
-            config = rewire('../../../../core/shared/config');
+            loader = rewire('../../../../core/shared/config/loader');
 
             // we manually call `loadConf` in the tests and we need to ensure that the minimum
             // required config properties are available
@@ -37,7 +37,7 @@ describe('Config', function () {
         it('env parameter is stronger than file', function () {
             process.env.database__client = 'test';
 
-            customConfig = config.loadNconf({
+            customConfig = loader.loadNconf({
                 baseConfigPath: path.join(__dirname, '../../../utils/fixtures/config'),
                 customConfigPath: path.join(__dirname, '../../../utils/fixtures/config')
             });
@@ -49,7 +49,7 @@ describe('Config', function () {
             process.env.database__client = 'test';
             process.argv[2] = '--database:client=stronger';
 
-            customConfig = config.loadNconf({
+            customConfig = loader.loadNconf({
                 baseConfigPath: path.join(__dirname, '../../../utils/fixtures/config'),
                 customConfigPath: path.join(__dirname, '../../../utils/fixtures/config')
             });
@@ -61,7 +61,7 @@ describe('Config', function () {
             process.env.paths__corePath = 'try-to-override';
             process.argv[2] = '--paths:corePath=try-to-override';
 
-            customConfig = config.loadNconf({
+            customConfig = loader.loadNconf({
                 baseConfigPath: path.join(__dirname, '../../../utils/fixtures/config'),
                 customConfigPath: path.join(__dirname, '../../../utils/fixtures/config')
             });
@@ -70,7 +70,7 @@ describe('Config', function () {
         });
 
         it('overrides is stronger than every other config file', function () {
-            customConfig = config.loadNconf({
+            customConfig = loader.loadNconf({
                 baseConfigPath: path.join(__dirname, '../../../utils/fixtures/config'),
                 customConfigPath: path.join(__dirname, '../../../utils/fixtures/config')
             });
@@ -124,56 +124,4 @@ describe('Config', function () {
             configUtils.config.getContentPath('images').should.eql(contentPath + 'images/');
         });
     });
-
-    describe('Storage', function () {
-        it('should default to local-file-store', function () {
-            configUtils.config.get('paths').should.have.property('internalAdaptersPath', path.join(configUtils.config.get('paths').corePath, '/server/adapters/'));
-
-            configUtils.config.get('storage').should.have.property('active', 'LocalFileStorage');
-        });
-
-        it('no effect: setting a custom active storage as string', function () {
-            configUtils.set({
-                storage: {
-                    active: 's3',
-                    s3: {}
-                }
-            });
-
-            configUtils.config.get('storage').should.have.property('active', 's3');
-            configUtils.config.get('storage').should.have.property('s3', {});
-        });
-
-        it('able to set storage for themes (but not officially supported!)', function () {
-            configUtils.set({
-                storage: {
-                    active: {
-                        images: 'local-file-store',
-                        themes: 's3'
-                    }
-                }
-            });
-
-            configUtils.config.get('storage').should.have.property('active', {
-                images: 'local-file-store',
-                themes: 's3'
-            });
-        });
-
-        it('should allow setting a custom active storage as object', function () {
-            configUtils.set({
-                storage: {
-                    active: {
-                        images: 's2',
-                        themes: 'local-file-store'
-                    }
-                }
-            });
-
-            configUtils.config.get('storage').should.have.property('active', {
-                images: 's2',
-                themes: 'local-file-store'
-            });
-        });
-    });
 });

test/unit/shared/config/utils_spec.js

@@ -1,7 +1,7 @@
 const should = require('should');
 const configUtils = require('../../../../core/shared/config/utils');
 
-describe('UNIT: Config utils', function () {
+describe('Config Utils', function () {
     describe('makePathsAbsolute', function () {
         it('ensure we change paths only', function () {
             const changedKey = [];