Skip to content

Commit

Permalink
Moved apps to /services/ & moved individual tests (#9187)
Browse files Browse the repository at this point in the history
refs #9178

* Moved app handling code into services/apps
  - Apps is a service, that allows for the App lifecycle 
  - /server/apps = contains internal apps 
   - /server/services/apps = contains code for managing/handling app life cycle, providing the proxy, etc
* Split apps service tests into separate files
* Moved internal app tests into test folders
    - Problem: Not all the tests in apps were unit tests, yet they were treated like they were in Gruntfile.js
    - Unit tests now live in /test/unit/apps
    - Route tests now live in /test/functional/routes/apps
    - Gruntfile.js has been updated to match
* Switch api.read usage for settingsCache
* Add tests to cover the basic App lifecycle
* Simplify some of the init logic
  • Loading branch information
ErisDS authored Oct 30, 2017
1 parent 97beaf0 commit 882a236
Show file tree
Hide file tree
Showing 27 changed files with 785 additions and 675 deletions.
10 changes: 3 additions & 7 deletions Gruntfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -149,16 +149,14 @@ var overrides = require('./core/server/overrides'),
// #### All Unit tests
unit: {
src: [
'core/test/unit/**/*_spec.js',
'core/server/apps/**/tests/*_spec.js'
'core/test/unit/**/*_spec.js'
]
},

// #### All Integration tests
integration: {
src: [
'core/test/integration/**/*_spec.js',
'core/test/integration/*_spec.js'
'core/test/integration/**/*_spec.js'
]
},

Expand Down Expand Up @@ -187,8 +185,7 @@ var overrides = require('./core/server/overrides'),
coverage: {
// they can also have coverage generated for them & the order doesn't matter
src: [
'core/test/unit',
'core/server/apps'
'core/test/unit'
],
options: {
mask: '**/*_spec.js',
Expand All @@ -200,7 +197,6 @@ var overrides = require('./core/server/overrides'),
coverage_all: {
src: [
'core/test/integration',
'core/server/apps',
'core/test/functional',
'core/test/unit'
],
Expand Down
101 changes: 0 additions & 101 deletions core/server/apps/index.js

This file was deleted.

11 changes: 6 additions & 5 deletions core/server/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,22 +14,23 @@ require('./overrides');

// Module dependencies
var debug = require('ghost-ignition').debug('boot:init'),
// Config should be first require, as it triggers the initial load of the config files
config = require('./config'),
Promise = require('bluebird'),
i18n = require('./i18n'),
models = require('./models'),
permissions = require('./permissions'),
apps = require('./apps'),
auth = require('./auth'),
dbHealth = require('./data/db/health'),
xmlrpc = require('./services/xmlrpc'),
slack = require('./services/slack'),
GhostServer = require('./ghost-server'),
scheduling = require('./adapters/scheduling'),
settings = require('./settings'),
themes = require('./themes'),
utils = require('./utils');
utils = require('./utils'),

// Services that need initialisation
apps = require('./services/apps'),
xmlrpc = require('./services/xmlrpc'),
slack = require('./services/slack');

// ## Initialise Ghost
function init() {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

var _ = require('lodash'),
fs = require('fs'),
path = require('path'),
Expand Down
76 changes: 76 additions & 0 deletions core/server/services/apps/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
var debug = require('ghost-ignition').debug('services:apps'),
_ = require('lodash'),
Promise = require('bluebird'),
logging = require('../../logging'),
errors = require('../../errors'),
api = require('../../api'),
i18n = require('../../i18n'),
config = require('../../config'),
settingsCache = require('../../settings/cache'),
loader = require('./loader'),
// Internal APps are in config
internalApps = config.get('apps:internal'),
// Holds the available apps
availableApps = {};

function recordLoadedApp(name, loadedApp) {
// After loading the app, add it to our hash of loaded apps
availableApps[name] = loadedApp;
return loadedApp;
}

function saveInstalledApps(installedApps) {
debug('saving begin');
var currentInstalledApps = settingsCache.get('installed_apps'),
// Never save internal apps
updatedAppsInstalled = _.difference(_.uniq(installedApps.concat(currentInstalledApps)), internalApps);

if (_.difference(updatedAppsInstalled, currentInstalledApps).length === 0) {
debug('saving unneeded');
return new Promise.resolve();
}

debug('saving settings');
return api.settings.edit({settings: [{key: 'installed_apps', value: updatedAppsInstalled}]}, {context: {internal: true}});
}

module.exports = {
init: function () {
debug('init begin');
var activeApps = settingsCache.get('active_apps'),
installedApps = settingsCache.get('installed_apps'),
// Load means either activate, or install and activate
// We load all Active Apps, and all Internal Apps
appsToLoad = activeApps.concat(internalApps);

function loadApp(appName) {
// If internal or already installed, the app only needs activating
if (_.includes(internalApps, appName) || _.includes(installedApps, appName)) {
return loader.activateAppByName(appName).then(function (loadedApp) {
return recordLoadedApp(appName, loadedApp);
});
}

// Else first install, then activate the app
return loader.installAppByName(appName).then(function () {
return loader.activateAppByName(appName);
}).then(function (loadedApp) {
return recordLoadedApp(appName, loadedApp);
});
}

return Promise.map(appsToLoad, loadApp)
.then(function () {
// Save our installed apps to settings
return saveInstalledApps(_.keys(availableApps));
})
.catch(function (err) {
logging.error(new errors.GhostError({
err: err,
context: i18n.t('errors.apps.appWillNotBeLoaded.error'),
help: i18n.t('errors.apps.appWillNotBeLoaded.help')
}));
});
},
availableApps: availableApps
};
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@

var path = require('path'),
_ = require('lodash'),
Promise = require('bluebird'),
config = require('../../config'),
i18n = require('../../i18n'),

AppProxy = require('./proxy'),
config = require('../config'),
AppSandbox = require('./sandbox'),
AppDependencies = require('./dependencies'),
AppPermissions = require('./permissions'),
i18n = require('../i18n'),

loader;

function isInternalApp(name) {
Expand Down
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/packages').parsePackageJSON;
parsePackageJson = require('../../utils/packages').parsePackageJSON;

function AppPermissions(appPath) {
this.appPath = appPath;
Expand Down
10 changes: 5 additions & 5 deletions core/server/apps/proxy.js → core/server/services/apps/proxy.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
var _ = require('lodash'),
api = require('../api'),
helpers = require('../helpers/register'),
filters = require('../filters'),
i18n = require('../i18n'),
var _ = require('lodash'),
api = require('../../api'),
helpers = require('../../helpers/register'),
filters = require('../../filters'),
i18n = require('../../i18n'),
generateProxyFunctions;

generateProxyFunctions = function (name, permissions, isInternal) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@

var path = require('path'),
Module = require('module'),
i18n = require('../i18n'),
_ = require('lodash');
var path = require('path'),
Module = require('module'),
i18n = require('../../i18n'),
_ = require('lodash');

function AppSandbox(opts) {
this.opts = _.defaults(opts || {}, AppSandbox.defaults);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
var supertest = require('supertest'),
should = require('should'),
sinon = require('sinon'),
testUtils = require('../../../../test/utils'),
labs = require('../../../utils/labs'),
config = require('../../../config'),
testUtils = require('../../../../utils'),
labs = require('../../../../../server/utils/labs'),
config = require('../../../../../server/config'),
ghost = testUtils.startGhost,
sandbox = sinon.sandbox.create();

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
var should = require('should'),

// Stuff we are testing
ampComponentsHelper = require('../lib/helpers/amp_components');
ampComponentsHelper = require('../../../../server/apps/amp/lib/helpers/amp_components');

describe('{{amp_components}} helper', function () {
it('adds script tag for a gif', function () {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ var should = require('should'),
configUtils = require('../../../../test/utils/configUtils'),

// Stuff we are testing
ampContentHelper = rewire('../lib/helpers/amp_content');
ampContentHelper = rewire('../../../../server/apps/amp/lib/helpers/amp_content');

// TODO: Amperize really needs to get stubbed, so we can test returning errors
// properly and make this test faster!
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ var should = require('should'),
path = require('path'),
Promise = require('bluebird'),

ampController = rewire('../lib/router'),
errors = require('../../../errors'),
configUtils = require('../../../../test/utils/configUtils'),
themes = require('../../../themes'),
ampController = rewire('../../../../server/apps/amp/lib/router'),
errors = require('../../../../server/errors'),
configUtils = require('../../../utils/configUtils'),
themes = require('../../../../server/themes'),

sandbox = sinon.sandbox.create();

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
var should = require('should'), // jshint ignore:line
card = require('../cards/hr'),
card = require('../../../../server/apps/default-cards/cards/hr'),
SimpleDom = require('simple-dom'),
opts;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
var should = require('should'), // jshint ignore:line
card = require('../cards/html'),
card = require('../../../../server/apps/default-cards/cards/html'),
SimpleDom = require('simple-dom'),
opts;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
var should = require('should'), // jshint ignore:line
card = require('../cards/image'),
card = require('../../../../server/apps/default-cards/cards/image'),
SimpleDom = require('simple-dom'),
opts;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
var should = require('should'), // jshint ignore:line
card = require('../cards/markdown'),
card = require('../../../../server/apps/default-cards/cards/markdown'),
SimpleDom = require('simple-dom'),
opts;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
var should = require('should'), // jshint ignore:line
card = require('../atoms/soft-return'),
card = require('../../../../server/apps/default-cards/atoms/soft-return'),
SimpleDom = require('simple-dom'),
opts;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
/*globals describe, beforeEach, afterEach, it*/
var should = require('should'),
sinon = require('sinon'),
configUtils = require('../../../../test/utils/configUtils'),
path = require('path'),
themes = require('../../../themes'),
privateController = require('../lib/router').controller,
configUtils = require('../../../utils/configUtils'),
themes = require('../../../../server/themes'),
privateController = require('../../../../server/apps/private-blogging/lib/router').controller,

sandbox = sinon.sandbox.create();

Expand Down
Loading

0 comments on commit 882a236

Please sign in to comment.