diff --git a/core/shared/config/loader.js b/core/shared/config/loader.js index 2ebd87139d..4f8271a61d 100644 --- a/core/shared/config/loader.js +++ b/core/shared/config/loader.js @@ -1,6 +1,4 @@ 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'); @@ -42,13 +40,6 @@ function loadNconf(options) { // ## Config Methods - // Bind internal-only methods - // @TODO change how we use these so we don't expose them - 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 helpers.bindAll(nconf); urlHelpers.bindAll(nconf); @@ -56,28 +47,16 @@ function loadNconf(options) { // ## Sanitization // transform all relative paths to absolute paths - nconf.makePathsAbsolute(nconf.get('paths'), 'paths'); + localUtils.makePathsAbsolute(nconf, 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())); - } - } + localUtils.sanitizeDatabaseProperties(nconf); // Check if the URL in config has a protocol - nconf.checkUrlProtocol(); + localUtils.checkUrlProtocol(nconf.get('url')); // Ensure that the content path exists - nconf.doesContentPathExist(); + localUtils.doesContentPathExist(nconf.get('paths:contentPath')); // ## Other Stuff! diff --git a/core/shared/config/utils.js b/core/shared/config/utils.js index e2ec49e625..91c679a9a5 100644 --- a/core/shared/config/utils.js +++ b/core/shared/config/utils.js @@ -1,5 +1,6 @@ const path = require('path'); const fs = require('fs-extra'); +const os = require('os'); const _ = require('lodash'); /** @@ -10,24 +11,22 @@ const _ = require('lodash'); * Path must match minimum one / or \ * Path can be a "." to re-present current folder */ -const makePathsAbsolute = function makePathsAbsolute(obj, parent) { - const self = this; - +const makePathsAbsolute = function makePathsAbsolute(nconf, obj, parent) { _.each(obj, function (configValue, pathsKey) { if (_.isObject(configValue)) { - makePathsAbsolute.bind(self)(configValue, parent + ':' + pathsKey); + makePathsAbsolute(nconf, configValue, parent + ':' + pathsKey); } else if ( _.isString(configValue) && (configValue.match(/\/+|\\+/) || configValue === '.') && !path.isAbsolute(configValue) ) { - self.set(parent + ':' + pathsKey, path.normalize(path.join(__dirname, '../../..', configValue))); + nconf.set(parent + ':' + pathsKey, path.normalize(path.join(__dirname, '../../..', configValue))); } }); }; -const doesContentPathExist = function doesContentPathExist() { - if (!fs.pathExistsSync(this.get('paths:contentPath'))) { +const doesContentPathExist = function doesContentPathExist(contentPath) { + if (!fs.pathExistsSync(contentPath)) { // new Error is allowed here, as we do not want config to depend on @tryghost/error // @TODO: revisit this decision when @tryghost/error is no longer dependent on all of ghost-ignition // eslint-disable-next-line no-restricted-syntax @@ -38,9 +37,7 @@ const doesContentPathExist = function doesContentPathExist() { /** * Check if the URL in config has a protocol and sanitise it if not including a warning that it should be changed */ -const checkUrlProtocol = function checkUrlProtocol() { - const url = this.get('url'); - +const checkUrlProtocol = function checkUrlProtocol(url) { if (!url.match(/^https?:\/\//i)) { // new Error is allowed here, as we do not want config to depend on @tryghost/error // @TODO: revisit this decision when @tryghost/error is no longer dependent on all of ghost-ignition @@ -56,10 +53,10 @@ const checkUrlProtocol = function checkUrlProtocol() { * this.clear('key') does not work * https://github.com/indexzero/nconf/issues/235#issuecomment-257606507 */ -const sanitizeDatabaseProperties = function sanitizeDatabaseProperties() { - const database = this.get('database'); +const sanitizeDatabaseProperties = function sanitizeDatabaseProperties(nconf) { + const database = nconf.get('database'); - if (this.get('database:client') === 'mysql') { + if (nconf.get('database:client') === 'mysql') { delete database.connection.filename; } else { delete database.connection.host; @@ -68,7 +65,19 @@ const sanitizeDatabaseProperties = function sanitizeDatabaseProperties() { delete database.connection.database; } - this.set('database', database); + nconf.set('database', database); + + if (nconf.get('database:client') === 'sqlite3') { + makePathsAbsolute(nconf, 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())); + } + } }; module.exports = { diff --git a/test/unit/shared/config/utils_spec.js b/test/unit/shared/config/utils_spec.js index 851418b9e5..015124e001 100644 --- a/test/unit/shared/config/utils_spec.js +++ b/test/unit/shared/config/utils_spec.js @@ -1,25 +1,34 @@ const should = require('should'); +const _ = require('lodash'); const configUtils = require('../../../../core/shared/config/utils'); +let fakeConfig = {}; +let fakeNconf = {}; +let changedKey = []; + describe('Config Utils', function () { describe('makePathsAbsolute', function () { - it('ensure we change paths only', function () { - const changedKey = []; + beforeEach(function () { + changedKey = []; - const obj = { - database: { - client: 'mysql', - connection: { - filename: 'content/data/ghost.db' - } + fakeNconf.get = (key) => { + key = key.replace(':', ''); + return _.get(fakeConfig, key); + }; + fakeNconf.set = function (key, value) { + changedKey.push([key, value]); + }; + }); + + it('ensure we change paths only', function () { + fakeConfig.database = { + client: 'mysql', + connection: { + filename: 'content/data/ghost.db' } }; - this.set = function (key, value) { - changedKey.push([key, value]); - }; - - configUtils.makePathsAbsolute.bind(this)(obj.database, 'database'); + configUtils.makePathsAbsolute(fakeNconf, fakeConfig.database, 'database'); changedKey.length.should.eql(1); changedKey[0][0].should.eql('database:connection:filename'); @@ -27,56 +36,33 @@ describe('Config Utils', function () { }); it('ensure it skips non strings', function () { - const changedKey = []; - - const obj = { - database: { - test: 10 - } + fakeConfig.database = { + test: 10 }; - this.set = function (key, value) { - changedKey.push([key, value]); - }; - - configUtils.makePathsAbsolute.bind(this)(obj.database, 'database'); + configUtils.makePathsAbsolute(fakeNconf, fakeConfig.database, 'database'); changedKey.length.should.eql(0); }); it('ensure we don\'t change absolute paths', function () { - const changedKey = []; - - const obj = { - database: { - client: 'mysql', - connection: { - filename: '/content/data/ghost.db' - } + fakeConfig.database = { + client: 'mysql', + connection: { + filename: '/content/data/ghost.db' } }; - this.set = function (key, value) { - changedKey.push([key, value]); - }; - - configUtils.makePathsAbsolute.bind(this)(obj.database, 'database'); + configUtils.makePathsAbsolute(fakeNconf, fakeConfig.database, 'database'); changedKey.length.should.eql(0); }); it('match paths on windows', function () { - const changedKey = []; + fakeConfig.database = { + filename: 'content\\data\\ghost.db' - const obj = { - database: { - filename: 'content\\data\\ghost.db' - } }; - this.set = function (key, value) { - changedKey.push([key, value]); - }; - - configUtils.makePathsAbsolute.bind(this)(obj.database, 'database'); + configUtils.makePathsAbsolute(fakeNconf, fakeConfig.database, 'database'); changedKey.length.should.eql(1); changedKey[0][0].should.eql('database:filename'); changedKey[0][1].should.not.eql('content\\data\\ghost.db');