mirror of
https://github.com/TryGhost/Ghost.git
synced 2024-12-01 05:50:35 +03:00
Removed bind from internal-only config helpers
- We were using the same bind pattern for both internal-only and public helpers - Binding helpers to config makes them available throughout the codebase - Removing the binding doesn't make the code much more complicated, but it does make the Public API of the config module a lot clearer
This commit is contained in:
Hannah Wolfe
parent
8cf411e524
commit
16b5d14c9c
@ -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!
|
||||
|
||||
|
||||
@ -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 = {
|
||||
|
||||
@ -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');
|
||||
|
||||
Loading…
Reference in New Issue
Block a user