Skip to content

Commit

Permalink
🎨 Collect & simplify package utils (#8080)
Browse files Browse the repository at this point in the history
closes #8056

🎨 Collect together the package-related utils
- read directory actually reads a directory of packages
- parse package json is very tighly related to this

🎨 Move filterPaths -> packages.filterPackages
- this function is related to packages, not settings
- move the function to the new utils/packages
- add 100% test coverage

🎨 Simplify filterPackages code
🎨 Simplify reading of packages & themes
- This massively reduces all the complex code in the read packages & themes utils
- Added full test coverage

🎨 Improve & clarify active prop in filterPackages
- active is returned from API endpoints to combine data from multiple sources
- see #8064 (comment)

🎨 Better error handling
🔥 Temporarily remove custom error templates
- we will reimplement this later when we have got a better concept of loading the active theme in place
- refs #8079
  • Loading branch information
ErisDS authored and kirrg001 committed Mar 1, 2017
1 parent fa38257 commit c70fbc2
Show file tree
Hide file tree
Showing 13 changed files with 747 additions and 477 deletions.
48 changes: 2 additions & 46 deletions core/server/api/settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ var _ = require('lodash'),
errors = require('../errors'),
utils = require('./utils'),
i18n = require('../i18n'),
filterPackages = require('../utils/packages').filterPackages,

docName = 'settings',
settings,
Expand All @@ -16,7 +17,6 @@ var _ = require('lodash'),

updateSettingsCache,
settingsFilter,
filterPaths,
readSettingsResult,
settingsResult,
canEditAllSettings,
Expand Down Expand Up @@ -66,50 +66,6 @@ settingsFilter = function (settings, filter) {
}));
};

/**
* ### Filter Paths
* Normalizes paths read by require-tree so that the apps and themes modules can use them. Creates an empty
* array (res), and populates it with useful info about the read packages like name, whether they're active
* (comparison with the second argument), and if they have a package.json, that, otherwise false
* @private
* @param {object} paths as returned by require-tree()
* @param {array/string} active as read from the settings object
* @returns {Array} of objects with useful info about apps / themes
*/
filterPaths = function (paths, active) {
var pathKeys = Object.keys(paths),
res = [],
item;

// turn active into an array (so themes and apps can be checked the same)
if (!Array.isArray(active)) {
active = [active];
}

_.each(pathKeys, function (key) {
// do not include hidden files or _messages
if (key.indexOf('.') !== 0 &&
key !== '_messages' &&
key !== 'README.md'
) {
item = {
name: key
};
if (paths[key].hasOwnProperty('package.json')) {
item.package = paths[key]['package.json'];
} else {
item.package = false;
}

if (_.indexOf(active, key) !== -1) {
item.active = true;
}
res.push(item);
}
});
return res;
};

/**
* ### Read Settings Result
* @private
Expand All @@ -129,7 +85,7 @@ readSettingsResult = function (settingsModels) {

// @TODO: remove availableThemes from settings cache and create an endpoint to fetch themes
if (settings.activeTheme && themes) {
res = filterPaths(themes, settings.activeTheme.value);
res = filterPackages(themes, settings.activeTheme.value);

settings.availableThemes = {
key: 'availableThemes',
Expand Down
2 changes: 1 addition & 1 deletion core/server/apps/permissions.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
var fs = require('fs'),
Promise = require('bluebird'),
path = require('path'),
parsePackageJson = require('../utils/parse-package-json');
parsePackageJson = require('../utils/packages').parsePackageJSON;

function AppPermissions(appPath) {
this.appPath = appPath;
Expand Down
7 changes: 2 additions & 5 deletions core/server/middleware/error-handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,8 @@ _private.JSONErrorRenderer = function JSONErrorRenderer(err, req, res, /*jshint
};

_private.HTMLErrorRenderer = function HTMLErrorRender(err, req, res, /*jshint unused:false */ next) {
// @TODO reconsider this
var availableTheme = config.get('paths').availableThemes[req.app.get('activeTheme')] || {},
defaultTemplate = availableTheme['error.hbs'] ||
path.resolve(config.get('paths').adminViews, 'user-error.hbs') ||
'error',
// @TODO re-implement custom error templates see #8079
var defaultTemplate = path.resolve(config.get('paths').adminViews, 'user-error.hbs'),
templateData = {
message: err.message,
code: err.statusCode
Expand Down
54 changes: 13 additions & 41 deletions core/server/themes/read.js
Original file line number Diff line number Diff line change
@@ -1,55 +1,27 @@
/**
* Dependencies
* # Read Themes
*
* Util that wraps packages.read
*/
var packages = require('../utils/packages'),
logging = require('../logging'),
i18n = require('../i18n'),

var readDirectory = require('../utils').readDirectory,
Promise = require('bluebird'),
_ = require('lodash'),
join = require('path').join,
fs = require('fs'),

statFile = Promise.promisify(fs.stat),
readOneTheme,
readAllThemes;

readOneTheme = function readOneTheme(dir, name) {
var toRead = join(dir, name),
themes = {};

return readDirectory(toRead)
.then(function (tree) {
if (!_.isEmpty(tree)) {
themes[name] = tree;
}

return themes;
return packages
.read.one(dir, name)
.catch(function () {
// For now we return an empty object as this is not fatal unless the frontend of the blog is requested
logging.warn(i18n.t('errors.middleware.themehandler.missingTheme', {theme: name}));
return {};
});
};

readAllThemes = function readAllThemes(dir) {
var originalTree;

return readDirectory(dir)
.tap(function (tree) {
originalTree = tree;
})
.then(Object.keys)
.filter(function (file) {
var path = join(dir, file);

return statFile(path).then(function (stat) {
return stat.isDirectory();
});
})
.then(function (directories) {
var themes = {};

directories.forEach(function (name) {
themes[name] = originalTree[name];
});

return themes;
});
return packages.read.all(dir);
};

/**
Expand Down
1 change: 0 additions & 1 deletion core/server/utils/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,6 @@ utils = {
},

readCSV: require('./read-csv'),
readDirectory: require('./read-directory'),
removeOpenRedirectFromUrl: require('./remove-open-redirect-from-url'),
zipFolder: require('./zip-folder'),
generateAssetHash: require('./asset-hash'),
Expand Down
44 changes: 44 additions & 0 deletions core/server/utils/packages/filter-packages.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
var _ = require('lodash'),
notAPackageRegex = /^\.|_messages|README.md/i,
filterPackages;

/**
* ### Filter Packages
* Normalizes packages read by read-packages so that the apps and themes modules can use them.
* Iterates over each package and return an array of objects which are simplified representations of the package
* with 3 properties:
* - `name` - the package name
* - `package` - contents of the package.json or false if there isn't one
* - `active` - set to true if this package is active
* This data structure is used for listings of packages provided over the API and as such
* deliberately combines multiple sources of information in order to be efficient.
*
* TODO: simplify the package.json representation to contain only fields we use
*
* @param {object} packages as returned by read-packages
* @param {array/string} active as read from the settings object
* @returns {Array} of objects with useful info about apps / themes
*/
filterPackages = function filterPackages(packages, active) {
// turn active into an array (so themes and apps can be checked the same)
if (!Array.isArray(active)) {
active = [active];
}

return _.reduce(packages, function (result, pkg, key) {
var item = {};
if (!key.match(notAPackageRegex)) {
item = {
name: key,
package: pkg['package.json'] || false,
active: _.indexOf(active, key) !== -1
};

result.push(item);
}

return result;
}, []);
};

module.exports = filterPackages;
17 changes: 17 additions & 0 deletions core/server/utils/packages/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/**
* # Package Utils
*
* Ghost has / is in the process of gaining support for several different types of sub-packages:
* - Themes: have always been packages, but we're going to lean more heavily on npm & package.json in future
* - Adapters: an early version of apps, replace fundamental pieces like storage, will become npm modules
* - Apps: plugins that can be installed whilst Ghost is running & modify behaviour
* - More?
*
* These utils facilitate loading, reading, managing etc, packages from the file system.
*/

module.exports = {
read: require('./read-packages'),
parsePackageJSON: require('./parse-package-json'),
filterPackages: require('./filter-packages')
};
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

var Promise = require('bluebird'),
fs = require('fs'),
i18n = require('../i18n'),
i18n = require('../../i18n'),

readFile = Promise.promisify(fs.readFile);

Expand Down
89 changes: 89 additions & 0 deletions core/server/utils/packages/read-packages.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
/**
* Dependencies
*/

var parsePackageJson = require('./parse-package-json'),
Promise = require('bluebird'),
_ = require('lodash'),
join = require('path').join,
fs = require('fs'),

notAPackageRegex = /^\.|_messages|README.md|node_modules|bower_components/i,
packageJSONPath = 'package.json',

statFile = Promise.promisify(fs.stat),
readDir = Promise.promisify(fs.readdir),

readPackage,
readPackages,
processPackage;

/**
* Recursively read directory and find the packages in it
*/
processPackage = function processPackage(absolutePath, packageName) {
var pkg = {
name: packageName,
path: absolutePath
};
return parsePackageJson(join(absolutePath, packageJSONPath))
.then(function gotPackageJSON(packageJSON) {
pkg['package.json'] = packageJSON;
return pkg;
})
.catch(function noPackageJSON() {
// ignore invalid package.json for now,
// because Ghost does not rely/use them at the moment
// in the future, this .catch() will need to be removed,
// so that error is thrown on invalid json syntax
pkg['package.json'] = null;
return pkg;
});
};

readPackage = function readPackage(packagePath, packageName) {
var absolutePath = join(packagePath, packageName);
return statFile(absolutePath)
.then(function (stat) {
if (!stat.isDirectory()) {
return {};
}

return processPackage(absolutePath, packageName)
.then(function gotPackage(pkg) {
var res = {};
res[packageName] = pkg;
return res;
});
})
.catch(function () {
return Promise.reject(new Error('Package not found'));
});
};

readPackages = function readPackages(packagePath) {
return readDir(packagePath)
.filter(function (packageName) {
// Filter out things which are not packages by regex
if (packageName.match(notAPackageRegex)) {
return;
}
// Check the remaining items to ensure they are a directory
return statFile(join(packagePath, packageName)).then(function (stat) {
return stat.isDirectory();
});
})
.map(function readPackageJson(packageName) {
var absolutePath = join(packagePath, packageName);
return processPackage(absolutePath, packageName);
})
.then(function (packages) {
return _.keyBy(packages, 'name');
});
};

/**
* Expose Public API
*/
module.exports.all = readPackages;
module.exports.one = readPackage;

0 comments on commit c70fbc2

Please sign in to comment.