From 20fec74c733a9e45a9c78202eb56dbfb44323f19 Mon Sep 17 00:00:00 2001 From: vdemedes Date: Tue, 6 Oct 2015 15:36:56 +0200 Subject: [PATCH] Refactor require-tree and split it into models closes #5492 - remove core/server/require-tree.js and split it into modules - add read-directory module to recursively read directories - add validate-themes module to scan themes and return errors/warnings - add parse-package-json module to parse json and validate requirements - rewrite core/server/models/index.js to manually require models --- core/server/apps/permissions.js | 16 +-- core/server/config/index.js | 4 +- core/server/data/validation/index.js | 4 +- core/server/index.js | 18 ++-- core/server/models/index.js | 104 +++++++++--------- core/server/require-tree.js | 137 ------------------------ core/server/utils/parse-package-json.js | 54 ++++++++++ core/server/utils/read-directory.js | 82 ++++++++++++++ core/server/utils/validate-themes.js | 52 +++++++++ 9 files changed, 261 insertions(+), 210 deletions(-) delete mode 100644 core/server/require-tree.js create mode 100644 core/server/utils/parse-package-json.js create mode 100644 core/server/utils/read-directory.js create mode 100644 core/server/utils/validate-themes.js diff --git a/core/server/apps/permissions.js b/core/server/apps/permissions.js index f66531dc41..140a83d9a7 100644 --- a/core/server/apps/permissions.js +++ b/core/server/apps/permissions.js @@ -2,7 +2,7 @@ var fs = require('fs'), Promise = require('bluebird'), path = require('path'), - parsePackageJson = require('../require-tree').parsePackageJson; + parsePackageJson = require('../utils/parse-package-json').parsePackageJson; function AppPermissions(appPath) { this.appPath = appPath; @@ -46,19 +46,7 @@ AppPermissions.prototype.checkPackageContentsExists = function () { // Get the contents of the package.json in the appPath root AppPermissions.prototype.getPackageContents = function () { - var messages = { - errors: [], - warns: [] - }; - - return parsePackageJson(this.packagePath, messages) - .then(function (parsed) { - if (!parsed) { - return Promise.reject(new Error(messages.errors[0].message)); - } - - return parsed; - }); + return parsePackageJson(this.packagePath); }; // Default permissions for an App. diff --git a/core/server/config/index.js b/core/server/config/index.js index f06d96a04c..68a10065a8 100644 --- a/core/server/config/index.js +++ b/core/server/config/index.js @@ -9,7 +9,7 @@ var path = require('path'), _ = require('lodash'), knex = require('knex'), validator = require('validator'), - requireTree = require('../require-tree').readAll, + readDirectory = require('../utils/read-directory'), errors = require('../errors'), configUrl = require('./url'), packageInfo = require('../../../package.json'), @@ -75,7 +75,7 @@ ConfigManager.prototype.init = function (rawConfig) { // just the object appropriate for this NODE_ENV self.set(rawConfig); - return Promise.all([requireTree(self._config.paths.themePath), requireTree(self._config.paths.appPath)]).then(function (paths) { + return Promise.all([readDirectory(self._config.paths.themePath), readDirectory(self._config.paths.appPath)]).then(function (paths) { self._config.paths.availableThemes = paths[0]; self._config.paths.availableApps = paths[1]; return self._config; diff --git a/core/server/data/validation/index.js b/core/server/data/validation/index.js index 8182a3d9ce..97115c4b62 100644 --- a/core/server/data/validation/index.js +++ b/core/server/data/validation/index.js @@ -4,7 +4,7 @@ var schema = require('../schema').tables, Promise = require('bluebird'), errors = require('../../errors'), config = require('../../config'), - requireTree = require('../../require-tree').readAll, + readDirectory = require('../../utils/read-directory'), validateSchema, validateSettings, @@ -112,7 +112,7 @@ validateActiveTheme = function validateActiveTheme(themeName) { // A Promise that will resolve to an object with a property for each installed theme. // This is necessary because certain configuration data is only available while Ghost // is running and at times the validations are used when it's not (e.g. tests) - availableThemes = requireTree(config.paths.themePath); + availableThemes = readDirectory(config.paths.themePath); } return availableThemes.then(function then(themes) { diff --git a/core/server/index.js b/core/server/index.js index e1d7e6a263..657b34bfe3 100644 --- a/core/server/index.js +++ b/core/server/index.js @@ -7,7 +7,6 @@ var express = require('express'), compress = require('compression'), fs = require('fs'), uuid = require('node-uuid'), - _ = require('lodash'), Promise = require('bluebird'), i18n = require('./i18n'), @@ -24,6 +23,7 @@ var express = require('express'), sitemap = require('./data/xml/sitemap'), xmlrpc = require('./data/xml/xmlrpc'), GhostServer = require('./ghost-server'), + validateThemes = require('./utils/validate-themes'), dbHash; @@ -200,13 +200,17 @@ function init(options) { middleware(blogApp, adminApp); // Log all theme errors and warnings - _.each(config.paths.availableThemes._messages.errors, function (error) { - errors.logError(error.message, error.context, error.help); - }); + validateThemes(config.paths.themePath) + .catch(function (result) { + // TODO: change `result` to something better + result.errors.forEach(function (err) { + errors.logError(err.message, err.context, err.help); + }); - _.each(config.paths.availableThemes._messages.warns, function (warn) { - errors.logWarn(warn.message, warn.context, warn.help); - }); + result.warnings.forEach(function (warn) { + errors.logWarn(warn.message, warn.context, warn.help); + }); + }); return new GhostServer(blogApp); }); diff --git a/core/server/models/index.js b/core/server/models/index.js index 1b7864ca5b..533da0d2f7 100644 --- a/core/server/models/index.js +++ b/core/server/models/index.js @@ -1,61 +1,69 @@ -var _ = require('lodash'), - Promise = require('bluebird'), - requireTree = require('../require-tree'), +/** + * Dependencies + */ + +var Promise = require('bluebird'), + _ = require('lodash'), + + exports, models; -models = { - excludeFiles: ['_messages', 'base', 'index.js'], +/** + * Expose all models + */ - // ### init - // Scan all files in this directory and then require each one and cache - // the objects exported onto this `models` object so that every other - // module can safely access models without fear of introducing circular - // dependency issues. - // @returns {Promise} - init: function init() { - var self = this; +exports = module.exports; - // One off inclusion of Base file. - self.Base = require('./base'); +models = [ + 'accesstoken', + 'app-field', + 'app-setting', + 'app', + 'client-trusted-domain', + 'client', + 'permission', + 'post', + 'refreshtoken', + 'role', + 'settings', + 'tag', + 'user' +]; - // Require all files in this directory - return requireTree.readAll(__dirname, {followSymlinks: false}).then(function then(modelFiles) { - // For each found file, excluding those we don't want, - // we will require it and cache it here. - _.each(modelFiles, function each(path, fileName) { - // Return early if this fileName is one of the ones we want - // to exclude. - if (_.contains(self.excludeFiles, fileName)) { - return; - } +function init() { + exports.Base = require('./base'); - // Require the file. - var file = require(path); + models.forEach(function (name) { + _.extend(exports, require('./' + name)); + }); - // Cache its `export` object onto this object. - _.extend(self, file); - }); + return Promise.resolve(); +} - return; - }); - }, - // ### deleteAllContent - // Delete all content from the database (posts, tags, tags_posts) - deleteAllContent: function deleteAllContent() { - var self = this; +/** + * TODO: move to some other place + */ - return self.Post.findAll().then(function then(posts) { - return Promise.all(_.map(posts.toJSON(), function mapper(post) { - return self.Post.destroy({id: post.id}); +// ### deleteAllContent +// Delete all content from the database (posts, tags, tags_posts) +exports.deleteAllContent = function deleteAllContent() { + var self = this; + + return self.Post.findAll().then(function then(posts) { + return Promise.all(_.map(posts.toJSON(), function mapper(post) { + return self.Post.destroy({id: post.id}); + })); + }).then(function () { + return self.Tag.findAll().then(function then(tags) { + return Promise.all(_.map(tags.toJSON(), function mapper(tag) { + return self.Tag.destroy({id: tag.id}); })); - }).then(function () { - return self.Tag.findAll().then(function then(tags) { - return Promise.all(_.map(tags.toJSON(), function mapper(tag) { - return self.Tag.destroy({id: tag.id}); - })); - }); }); - } + }); }; -module.exports = models; +/** + * Expose `init` + */ + +exports.init = init; diff --git a/core/server/require-tree.js b/core/server/require-tree.js deleted file mode 100644 index 7d2b75776c..0000000000 --- a/core/server/require-tree.js +++ /dev/null @@ -1,137 +0,0 @@ -// # Require Tree -// Require a tree of directories - used for loading themes and other things -var _ = require('lodash'), - fs = require('fs'), - path = require('path'), - Promise = require('bluebird'), - readdirAsync = Promise.promisify(fs.readdir), - lstatAsync = Promise.promisify(fs.lstat), - readlinkAsync = Promise.promisify(fs.readlink), - - parsePackageJson = function (path, messages) { - // Default the messages if non were passed - messages = messages || { - errors: [], - warns: [] - }; - - var jsonContainer; - - return new Promise(function (resolve) { - fs.readFile(path, function (error, data) { - if (error) { - messages.errors.push({ - message: 'Could not read package.json file', - context: path - }); - resolve(false); - return; - } - try { - jsonContainer = JSON.parse(data); - if (jsonContainer.hasOwnProperty('name') && jsonContainer.hasOwnProperty('version')) { - resolve(jsonContainer); - } else { - messages.errors.push({ - message: '"name" or "version" is missing from theme package.json file.', - context: path, - help: 'This will be required in future. Please see http://docs.ghost.org/themes/' - }); - resolve(false); - } - } catch (e) { - messages.errors.push({ - message: 'Theme package.json file is malformed', - context: path, - help: 'This will be required in future. Please see http://docs.ghost.org/themes/' - }); - resolve(false); - } - }); - }); - }, - - readDir = function (dir, options, depth, messages) { - depth = depth || 0; - messages = messages || { - errors: [], - warns: [] - }; - - options = _.extend({ - index: true, - followSymlinks: true - }, options); - - if (depth > 1) { - return Promise.resolve(null); - } - - return readdirAsync(dir).then(function (files) { - files = files || []; - - return Promise.reduce(files, function (results, file) { - var fpath = path.join(dir, file); - - return lstatAsync(fpath).then(function (result) { - if (result.isDirectory()) { - return readDir(fpath, options, depth + 1, messages); - } else if (options.followSymlinks && result.isSymbolicLink()) { - return readlinkAsync(fpath).then(function (linkPath) { - linkPath = path.resolve(dir, linkPath); - - return lstatAsync(linkPath).then(function (result) { - if (result.isFile()) { - return linkPath; - } - - return readDir(linkPath, options, depth + 1, messages); - }); - }); - } else if (depth === 1 && file === 'package.json') { - return parsePackageJson(fpath, messages); - } else { - return fpath; - } - }).then(function (result) { - results[file] = result; - - return results; - }); - }, {}); - }); - }, - readAll = function (dir, options, depth) { - // Start with clean messages, pass down along traversal - var messages = { - errors: [], - warns: [] - }; - - return readDir(dir, options, depth, messages).then(function (paths) { - // for all contents of the dir, I'm interested in the ones that are directories and within /theme/ - if (typeof paths === 'object' && dir.indexOf('theme') !== -1) { - _.each(paths, function (path, index) { - if (typeof path === 'object' && !path.hasOwnProperty('package.json') && index.indexOf('.') !== 0) { - messages.warns.push({ - message: 'Found a theme with no package.json file', - context: 'Theme name: ' + index, - help: 'This will be required in future. Please see http://docs.ghost.org/themes/' - }); - } - }); - } - - paths._messages = messages; - - return paths; - }).catch(function () { - return {_messages: messages}; - }); - }; - -module.exports = { - readAll: readAll, - readDir: readDir, - parsePackageJson: parsePackageJson -}; diff --git a/core/server/utils/parse-package-json.js b/core/server/utils/parse-package-json.js new file mode 100644 index 0000000000..2ef595ac24 --- /dev/null +++ b/core/server/utils/parse-package-json.js @@ -0,0 +1,54 @@ +/** + * Dependencies + */ + +var Promise = require('bluebird'), + fs = require('fs'), + + readFile = Promise.promisify(fs.readFile); + +/** + * Parse package.json and validate it has + * all the required fields + */ + +function parsePackageJson(path) { + return readFile(path) + .catch(function () { + var err = new Error('Could not read package.json file'); + err.context = path; + + return Promise.reject(err); + }) + .then(function (source) { + var hasRequiredKeys, json, err; + + try { + json = JSON.parse(source); + + hasRequiredKeys = json.name && json.version; + + if (!hasRequiredKeys) { + err = new Error('"name" or "version" is missing from theme package.json file.'); + err.context = path; + err.help = 'This will be required in future. Please see http://docs.ghost.org/themes/'; + + return Promise.reject(err); + } + + return json; + } catch (_) { + err = new Error('Theme package.json file is malformed'); + err.context = path; + err.help = 'This will be required in future. Please see http://docs.ghost.org/themes/'; + + return Promise.reject(err); + } + }); +} + +/** + * Expose `parsePackageJson` + */ + +module.exports = parsePackageJson; diff --git a/core/server/utils/read-directory.js b/core/server/utils/read-directory.js new file mode 100644 index 0000000000..415e834c48 --- /dev/null +++ b/core/server/utils/read-directory.js @@ -0,0 +1,82 @@ +/** + * Dependencies + */ + +var parsePackageJson = require('./parse-package-json'), + Promise = require('bluebird'), + join = require('path').join, + fs = require('fs'), + + statFile = Promise.promisify(fs.stat), + readDir = Promise.promisify(fs.readdir); + +/** + * Recursively read directory + */ + +function readDirectory(dir, options) { + var ignore; + + if (!options) { + options = {}; + } + + ignore = options.ignore || []; + ignore.push('node_modules', 'bower_components'); + + return readDir(dir) + .filter(function (filename) { + return ignore.indexOf(filename) === -1; + }) + .map(function (filename) { + var absolutePath = join(dir, filename); + + return statFile(absolutePath).then(function (stat) { + var item = { + name: filename, + path: absolutePath, + stat: stat + }; + + return item; + }); + }) + .map(function (item) { + if (item.name === 'package.json') { + return parsePackageJson(item.path).then(function (pkg) { + item.content = pkg; + + return item; + }); + } + + if (item.stat.isDirectory()) { + return readDirectory(item.path).then(function (files) { + item.content = files; + + return item; + }); + } + + // if there's no custom handling needed + // set absolute path as `item`'s `content` + item.content = item.path; + + return item; + }) + .then(function (items) { + var tree = {}; + + items.forEach(function (item) { + tree[item.name] = item.content; + }); + + return tree; + }); +} + +/** + * Expose `readDirectory` + */ + +module.exports = readDirectory; diff --git a/core/server/utils/validate-themes.js b/core/server/utils/validate-themes.js new file mode 100644 index 0000000000..72644d18fd --- /dev/null +++ b/core/server/utils/validate-themes.js @@ -0,0 +1,52 @@ +/** + * Dependencies + */ + +var readDirectory = require('./read-directory'), + Promise = require('bluebird'), + _ = require('lodash'); + +/** + * Validate themes: + * + * 1. Check if theme has package.json + */ + +function validateThemes(dir) { + var result = { + warnings: [], + errors: [] + }; + + return readDirectory(dir) + .tap(function (themes) { + _.each(themes, function (theme, name) { + var hasPackageJson, warning; + + hasPackageJson = !!theme['package.json']; + + if (!hasPackageJson) { + warning = { + message: 'Found a theme with no package.json file', + context: 'Theme name: ' + name, + help: 'This will be required in future. Please see http://docs.ghost.org/themes/' + }; + + result.warnings.push(warning); + } + }); + }) + .then(function () { + var hasNotifications = result.warnings.length || result.errors.length; + + if (hasNotifications) { + return Promise.reject(result); + } + }); +} + +/** + * Expose `validateThemes` + */ + +module.exports = validateThemes;