From c000435be96aaf9d2f9790f05d2752990f947d60 Mon Sep 17 00:00:00 2001
From: Raymond Feng <rfeng@us.ibm.com>
Date: Tue, 31 May 2016 16:43:58 -0700
Subject: [PATCH] Tidy up the iteration per review comment
---
lib/bootstrapper.js | 63 ++++++++++++++++++-----------------
lib/bundler.js | 10 ++++++
lib/plugin-loader.js | 2 --
lib/plugins/application.js | 20 +----------
package.json | 17 +++++-----
test/bootstrapper.test.js | 27 +++++++++------
test/browser.multiapp.test.js | 10 ------
test/browser.test.js | 10 ------
test/compiler.test.js | 45 +++++++++++++++----------
test/executor.test.js | 39 ++++++++++++----------
10 files changed, 119 insertions(+), 124 deletions(-)
diff --git a/lib/bootstrapper.js b/lib/bootstrapper.js
index 1ac91da..a159bc8 100644
--- a/lib/bootstrapper.js
+++ b/lib/bootstrapper.js
@@ -144,6 +144,37 @@ Bootstrapper.prototype.addPhases = function(phases) {
return this.phases;
};
+function pluginIteratorFactory(context, phase) {
+ return function(plugin, done) {
+ var result;
+ if (typeof plugin.handler[phase] !== 'function') {
+ debug('Skipping %s.%s', plugin.handler.name, phase);
+ return done();
+ }
+ debug('Invoking %s.%s', plugin.handler.name, phase);
+ try {
+ if (plugin.handler[phase].length === 2) {
+ plugin.handler[phase](context, done);
+ } else {
+ result = plugin.handler[phase](context);
+ if (typeof Promise !== 'undefined') {
+ Promise.resolve(result).then(function(value) {
+ done(null, value);
+ }).catch(function(err) {
+ debug('Unable to invoke %s.%s()', plugin.name, phase, err);
+ done(err);
+ });
+ } else {
+ done(null, result);
+ }
+ }
+ } catch (err) {
+ debug('Unable to invoke %s.%s()', plugin.name, phase, err);
+ done(err);
+ }
+ };
+}
+
/**
* Invoke the plugins phase by phase with the given context
* @param {Object} context Context object
@@ -168,37 +199,9 @@ Bootstrapper.prototype.run = function(context, done) {
var phases = context.phases || this.phases;
var bootPlugins = this.getExtensions('/boot');
- async.eachSeries(phases, function(phase, cb1) {
+ async.eachSeries(phases, function(phase, done) {
debug('Phase %s', phase);
- async.eachSeries(bootPlugins, function(plugin, cb2) {
- var result;
- if (typeof plugin.handler[phase] === 'function') {
- debug('Invoking %s.%s', plugin.handler.name, phase);
- try {
- if (plugin.handler[phase].length === 2) {
- plugin.handler[phase](context, cb2);
- } else {
- result = plugin.handler[phase](context);
- if (typeof Promise !== 'undefined') {
- Promise.resolve(result).then(function(value) {
- cb2(null, value);
- }).catch(function(err) {
- debug(err);
- cb2(err);
- });
- } else {
- cb2(null, result);
- }
- }
- } catch (err) {
- debug(err);
- cb2(err);
- }
- } else {
- debug('Skipping %s.%s', plugin.handler.name, phase);
- return cb2();
- }
- }, cb1);
+ async.eachSeries(bootPlugins, pluginIteratorFactory(context, phase), done);
}, function(err) {
return done(err, context);
});
diff --git a/lib/bundler.js b/lib/bundler.js
index d638fd7..114ffec 100644
--- a/lib/bundler.js
+++ b/lib/bundler.js
@@ -15,6 +15,7 @@ var g = require('strong-globalize')();
* @param {Object} bundler A browserify object created by `browserify()`.
*/
module.exports = function addInstructionsToBrowserify(context, bundler) {
+ addPlugins(bundler);
// bundlePluginScripts(context, bundler);
bundleModelScripts(context, bundler);
bundleMixinScripts(context, bundler);
@@ -23,6 +24,15 @@ module.exports = function addInstructionsToBrowserify(context, bundler) {
bundleInstructions(context, bundler);
};
+function addPlugins(bundler) {
+ var dir = path.join(__dirname, './plugins');
+ var files = fs.readdirSync(dir);
+ files.forEach(function(f) {
+ bundler.require(path.join(dir, f),
+ { expose: './plugins/' + path.basename(f, '.js') });
+ });
+}
+
function bundleOtherScripts(context, bundler) {
var list = context.instructions.bootScripts;
addScriptsToBundle('boot', list, bundler);
diff --git a/lib/plugin-loader.js b/lib/plugin-loader.js
index 227fd8e..3f11159 100644
--- a/lib/plugin-loader.js
+++ b/lib/plugin-loader.js
@@ -38,8 +38,6 @@ PluginScript.prototype.load = function(context) {
pluginScripts = pluginScripts.concat(utils.findScripts(envdir));
});
- // de-dedup boot scripts -ERS
- // https://github.com/strongloop/loopback-boot/issues/64
pluginScripts = _.uniq(pluginScripts);
debug('Plugin scripts: %j', pluginScripts);
this.configure(context, pluginScripts);
diff --git a/lib/plugins/application.js b/lib/plugins/application.js
index 47b34fb..9e12343 100644
--- a/lib/plugins/application.js
+++ b/lib/plugins/application.js
@@ -19,24 +19,8 @@ function Application(options) {
util.inherits(Application, PluginBase);
-function patchAppLoopback(app) {
- if (app.loopback) return;
- // app.loopback was introduced in 1.9.0
- // patch the app object to make loopback-boot work with older versions too
- try {
- app.loopback = require('loopback');
- } catch (err) {
- if (err.code === 'MODULE_NOT_FOUND') {
- console.error(
- 'When using loopback-boot with loopback <1.9, ' +
- 'the loopback module must be available for `require(\'loopback\')`.');
- }
- throw err;
- }
-}
-
function assertLoopBackVersion(app) {
- var RANGE = '1.x || 2.x || ^3.0.0-alpha';
+ var RANGE = '2.x || ^3.0.0-alpha';
var loopback = app.loopback;
// remove any pre-release tag from the version string,
@@ -130,8 +114,6 @@ function applyAppConfig(app, appConfig) {
Application.prototype.starting = function(context) {
var app = context.app;
app.booting = true;
-
- patchAppLoopback(app);
assertLoopBackVersion(app);
var appConfig = context.instructions.application;
diff --git a/package.json b/package.json
index 0ba7747..16da0c0 100644
--- a/package.json
+++ b/package.json
@@ -25,24 +25,25 @@
"license": "MIT",
"dependencies": {
"async": "^1.5.2",
- "bluebird": "^3.3.5",
+ "bluebird": "^3.4.0",
"commondir": "1.0.1",
"debug": "^2.2.0",
- "lodash": "^4.11.1",
+ "lodash": "^4.13.1",
"semver": "^5.1.0",
"strong-globalize": "^2.6.2",
- "toposort": "^0.2.12"
+ "toposort": "^1.0.0"
},
"devDependencies": {
"browserify": "^4.2.3",
- "eslint": "^2.5.3",
- "eslint-config-loopback": "^1.0.0",
"chai": "^3.5.0",
"coffee-script": "^1.10.0",
"coffeeify": "^2.0.1",
- "fs-extra": "^0.28.0",
- "loopback": "^2.27.0",
- "mocha": "^2.4.5",
+ "dirty-chai": "^1.2.2",
+ "eslint": "^2.11.1",
+ "eslint-config-loopback": "^1.0.0",
+ "fs-extra": "^0.30.0",
+ "loopback": "^2.28.0",
+ "mocha": "^2.5.3",
"supertest": "^1.2.0"
}
}
diff --git a/test/bootstrapper.test.js b/test/bootstrapper.test.js
index c649d38..7babf9f 100644
--- a/test/bootstrapper.test.js
+++ b/test/bootstrapper.test.js
@@ -1,6 +1,11 @@
var path = require('path');
var loopback = require('loopback');
-var expect = require('chai').expect;
+
+var chai = require('chai');
+var dirtyChai = require('dirty-chai');
+var expect = chai.expect;
+chai.use(dirtyChai);
+
var Bootstrapper = require('../lib/bootstrapper').Bootstrapper;
describe('Bootstrapper', function() {
@@ -25,12 +30,12 @@ describe('Bootstrapper', function() {
bootstrapper.run(context, function(err) {
if (err) return done(err);
- expect(context.configurations.application).to.be.object;
- expect(context.configurations.bootScripts).to.be.object;
- expect(context.configurations.middleware).to.be.object;
- expect(context.configurations.models).to.be.object;
+ expect(context.configurations.application).to.be.an('object');
+ expect(context.configurations.bootScripts).to.be.an('array');
+ expect(context.configurations.middleware).to.be.an('object');
+ expect(context.configurations.models).to.be.an('object');
expect(context.configurations.tracker).to.eql('load');
- expect(context.instructions).to.be.undefined;
+ expect(context.instructions).to.be.undefined();
expect(process.bootFlags.length).to.eql(0);
done();
});
@@ -51,11 +56,11 @@ describe('Bootstrapper', function() {
bootstrapper.run(context, function(err) {
if (err) return done(err);
- expect(context.configurations.application).to.be.object;
- expect(context.configurations.middleware).to.be.undefined;
- expect(context.configurations.models).to.be.undefined;
- expect(context.configurations.bootScripts).to.be.object;
- expect(context.instructions.application).to.be.object;
+ expect(context.configurations.application).to.be.an('object');
+ expect(context.configurations.middleware).to.be.undefined();
+ expect(context.configurations.models).to.be.undefined();
+ expect(context.configurations.bootScripts).to.be.an('array');
+ expect(context.instructions.application).to.be.an('object');
expect(context.instructions.tracker).to.eql('compile');
expect(context.executions.tracker).to.eql('start');
expect(process.bootFlags).to.eql(['barLoaded',
diff --git a/test/browser.multiapp.test.js b/test/browser.multiapp.test.js
index 5beb96e..d58ac61 100644
--- a/test/browser.multiapp.test.js
+++ b/test/browser.multiapp.test.js
@@ -60,22 +60,12 @@ describe('browser support for multiple apps', function() {
});
});
-function addPlugins(b) {
- var files = fs.readdirSync(path.join(__dirname, '../lib/plugins'));
- files.forEach(function(f) {
- b.require('../../lib/plugins/' + f,
- { expose: './plugins/' + path.basename(f, '.js') });
- });
-}
-
function browserifyTestApps(apps, next) {
var b = browserify({
debug: true,
basedir: path.resolve(__dirname, './fixtures'),
});
- addPlugins(b);
-
var bundles = [];
for (var i in apps) {
var appDir = apps[i].appDir;
diff --git a/test/browser.test.js b/test/browser.test.js
index 7857730..3f981b5 100644
--- a/test/browser.test.js
+++ b/test/browser.test.js
@@ -14,21 +14,12 @@ var vm = require('vm');
var createBrowserLikeContext = require('./helpers/browser').createContext;
var printContextLogs = require('./helpers/browser').printContextLogs;
-function addPlugins(b) {
- var files = fs.readdirSync(path.join(__dirname, '../lib/plugins'));
- files.forEach(function(f) {
- b.require('../../../lib/plugins/' + f,
- { expose: './plugins/' + path.basename(f, '.js') });
- });
-}
-
var compileStrategies = {
'default': function(appDir) {
var b = browserify({
basedir: appDir,
debug: true,
});
- addPlugins(b);
b.require('./app.js', { expose: 'browser-app' });
return b;
},
@@ -39,7 +30,6 @@ var compileStrategies = {
extensions: ['.coffee'],
debug: true,
});
- addPlugins(b);
b.transform('coffeeify');
b.require('./app.coffee', { expose: 'browser-app' });
diff --git a/test/compiler.test.js b/test/compiler.test.js
index 9241c61..5dd9385 100644
--- a/test/compiler.test.js
+++ b/test/compiler.test.js
@@ -19,7 +19,11 @@ describe('compiler', function() {
beforeEach(sandbox.reset);
beforeEach(appdir.init);
- function expectToThrow(err, done, options) {
+ function expectCompileToThrow(err, options, done) {
+ if (typeof options === 'function') {
+ done = options;
+ options = undefined;
+ }
boot.compile(options || appdir.PATH, function(err) {
expect(function() {
if (err) throw err;
@@ -28,7 +32,11 @@ describe('compiler', function() {
});
}
- function expectToNotThrow(done, options) {
+ function expectCompileToNotThrow(options, done) {
+ if (typeof options === 'function') {
+ done = options;
+ options = undefined;
+ }
boot.compile(options || appdir.PATH, function(err) {
expect(function() {
if (err) throw err;
@@ -570,7 +578,8 @@ describe('compiler', function() {
},
});
- expectToThrow(/array values of different length.*nest\.array/, done);
+ expectCompileToThrow(/array values of different length.*nest\.array/,
+ done);
});
it('refuses to merge Array of different length in Array', function(done) {
@@ -582,7 +591,7 @@ describe('compiler', function() {
key: [['value']],
});
- expectToThrow(/array values of different length.*key\[0\]/, done);
+ expectCompileToThrow(/array values of different length.*key\[0\]/, done);
});
it('returns full key of an incorrect Array value', function(done) {
@@ -602,7 +611,8 @@ describe('compiler', function() {
],
});
- expectToThrow(/array values of different length.*toplevel\[0\]\.nested/,
+ expectCompileToThrow(
+ /array values of different length.*toplevel\[0\]\.nested/,
done);
});
@@ -614,7 +624,7 @@ describe('compiler', function() {
key: {},
});
- expectToThrow(/incompatible types.*key/, done);
+ expectCompileToThrow(/incompatible types.*key/, done);
});
it('refuses to merge incompatible array items', function(done) {
@@ -625,7 +635,7 @@ describe('compiler', function() {
key: [{}],
});
- expectToThrow(/incompatible types.*key\[0\]/, done);
+ expectCompileToThrow(/incompatible types.*key\[0\]/, done);
});
it('merges app configs from multiple files', function(done) {
@@ -977,7 +987,7 @@ describe('compiler', function() {
foo: { properties: { name: 'string' }},
});
- expectToThrow(/unsupported 1\.x format/, done);
+ expectCompileToThrow(/unsupported 1\.x format/, done);
});
it('throws when model-config.json contains 1.x `options.base`',
@@ -986,7 +996,7 @@ describe('compiler', function() {
Customer: { options: { base: 'User' }},
});
- expectToThrow(/unsupported 1\.x format/, done);
+ expectCompileToThrow(/unsupported 1\.x format/, done);
});
it('loads models from `./models`', function(done) {
@@ -1341,7 +1351,7 @@ describe('compiler', function() {
base: 'Car',
});
- expectToThrow(/cyclic dependency/i, done);
+ expectCompileToThrow(/cyclic dependency/i, done);
});
it('uses file name as default value for model name', function(done) {
@@ -1841,8 +1851,8 @@ describe('compiler', function() {
it('throws error for invalid normalization format', function(done) {
options.normalization = 'invalidFormat';
- expectToThrow(/Invalid normalization format - "invalidFormat"/,
- done, options);
+ expectCompileToThrow(/Invalid normalization format - "invalidFormat"/,
+ options, done);
});
});
@@ -1985,7 +1995,7 @@ describe('compiler', function() {
},
});
- expectToThrow(/path-does-not-exist/, done);
+ expectCompileToThrow(/path-does-not-exist/, done);
});
it('does not fail when an optional middleware cannot be resolved',
@@ -1998,7 +2008,7 @@ describe('compiler', function() {
},
});
- expectToNotThrow(done);
+ expectCompileToNotThrow(done);
});
it('fails when a module middleware fragment cannot be resolved',
@@ -2009,7 +2019,7 @@ describe('compiler', function() {
},
});
- expectToThrow(/path-does-not-exist/, done);
+ expectCompileToThrow(/path-does-not-exist/, done);
});
it('does not fail when an optional middleware fragment cannot be resolved',
@@ -2022,7 +2032,7 @@ describe('compiler', function() {
},
});
- expectToNotThrow(done);
+ expectCompileToNotThrow(done);
});
it('resolves paths relatively to appRootDir', function(done) {
@@ -2778,7 +2788,8 @@ describe('compiler', function() {
});
appdir.writeConfigFileSync('./my-component/component.js', '');
- expectToThrow('Cannot resolve path \"my-component/component.js\"',
+ expectCompileToThrow(
+ 'Cannot resolve path \"my-component/component.js\"',
done);
});
diff --git a/test/executor.test.js b/test/executor.test.js
index b02f81b..c41f9f6 100644
--- a/test/executor.test.js
+++ b/test/executor.test.js
@@ -8,7 +8,12 @@ var boot = require('../');
var path = require('path');
var loopback = require('loopback');
var assert = require('assert');
-var expect = require('chai').expect;
+
+var chai = require('chai');
+var dirtyChai = require('dirty-chai');
+var expect = chai.expect;
+chai.use(dirtyChai);
+
var fs = require('fs-extra');
var sandbox = require('./helpers/sandbox');
var appdir = require('./helpers/appdir');
@@ -62,13 +67,13 @@ describe('executor', function() {
describe('when booting', function() {
it('should set the `booting` flag during execution', function(done) {
- expect(app.booting).to.be.undefined;
+ expect(app.booting).to.be.undefined();
simpleAppInstructions(function(err, context) {
if (err) return done(err);
boot.execute(app, context.instructions, function(err) {
expect(err).to.not.exist;
- expect(process.bootingFlagSet).to.be.true;
- expect(app.booting).to.be.false;
+ expect(process.bootingFlagSet).to.be.true();
+ expect(app.booting).to.be.false();
done();
});
});
@@ -410,16 +415,16 @@ describe('executor', function() {
});
it('should honor host and port', function(done) {
- function assertHonored(portKey, hostKey, done) {
+ function assertHonored(portKey, hostKey, cb) {
process.env[hostKey] = randomPort();
process.env[portKey] = randomHost();
bootWithDefaults(function(err) {
- if (err) return done(err);
+ if (err) return cb(err);
assert.equal(app.get('port'), process.env[portKey], portKey);
assert.equal(app.get('host'), process.env[hostKey], hostKey);
delete process.env[portKey];
delete process.env[hostKey];
- done();
+ cb();
});
}
@@ -430,8 +435,8 @@ describe('executor', function() {
{ port: 'OPENSHIFT_SLS_PORT', host: 'OPENSHIFT_SLS_IP' },
{ port: 'VCAP_APP_PORT', host: 'VCAP_APP_HOST' },
{ port: 'PORT', host: 'HOST' },
- ], function(config, done) {
- assertHonored(config.port, config.host, done);
+ ], function(config, cb) {
+ assertHonored(config.port, config.host, cb);
}, done);
});
@@ -656,8 +661,8 @@ describe('executor', function() {
supertest(app)
.get('/')
.end(function(err, res) {
- expect(err).to.be.null;
- expect(res.body.path).to.be.undefined;
+ expect(err).to.be.null();
+ expect(res.body.path).to.be.undefined();
cb();
});
}, cb);
@@ -675,7 +680,7 @@ describe('executor', function() {
supertest(app)
.get('/')
.end(function(err, res) {
- expect(err).to.be.null;
+ expect(err).to.be.null();
done();
});
});
@@ -833,7 +838,7 @@ describe('executor', function() {
boot.execute(app, someInstructions({ bootScripts: [file] }),
function(err) {
if (err) return done(err);
- expect(app.fnCalled, 'exported fn was called').to.be.true;
+ expect(app.fnCalled, 'exported fn was called').to.be.true();
done();
});
});
@@ -1023,12 +1028,12 @@ describe('executor', function() {
describe('when booting with env', function() {
it('should set the `booting` flag during execution', function(done) {
- expect(app.booting).to.be.undefined;
+ expect(app.booting).to.be.undefined();
envAppInstructions(function(err, context) {
if (err) return done(err);
boot.execute(app, context.instructions, function(err) {
if (err) return done(err);
- expect(app.booting).to.be.false;
+ expect(app.booting).to.be.false();
expect(process.bootFlags).to.not.have.property('barLoadedInTest');
done();
});
@@ -1156,8 +1161,8 @@ describe('executor', function() {
var bootInstructions = { dataSources: datasource };
boot.execute(app, someInstructions(bootInstructions), function() {
- expect(app.get('DYNAMIC_HOST')).to.be.undefined;
- expect(app.datasources.mydb.settings.host).to.be.undefined;
+ expect(app.get('DYNAMIC_HOST')).to.be.undefined();
+ expect(app.datasources.mydb.settings.host).to.be.undefined();
done();
});
});