Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .jshintrc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
"newcap": true,
"nonew": true,
"sub": true,
"unused": "vars",
"globals": {
"describe": true,
"it": true,
Expand Down
4 changes: 2 additions & 2 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,12 +75,12 @@ var addInstructionsToBrowserify = require('./lib/bundler');
* @header boot(app, [options])
*/

exports = module.exports = function bootLoopBackApp(app, options) {
exports = module.exports = function bootLoopBackApp(app, options, callback) {
// backwards compatibility with loopback's app.boot
options.env = options.env || app.get('env');

var instructions = compile(options);
execute(app, instructions);
execute(app, instructions, callback);
};

/**
Expand Down
51 changes: 41 additions & 10 deletions lib/executor.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ var assert = require('assert');
var _ = require('underscore');
var semver = require('semver');
var debug = require('debug')('loopback:boot:executor');
var async = require('async');

/**
* Execute bootstrap instructions gathered by `boot.compile`.
Expand All @@ -12,7 +13,7 @@ var debug = require('debug')('loopback:boot:executor');
* @header boot.execute(instructions)
*/

module.exports = function execute(app, instructions) {
module.exports = function execute(app, instructions, callback) {
patchAppLoopback(app);
assertLoopBackVersion(app);

Expand All @@ -24,9 +25,16 @@ module.exports = function execute(app, instructions) {
setupDataSources(app, instructions);
setupModels(app, instructions);

runBootScripts(app, instructions);

enableAnonymousSwagger(app, instructions);
// Run the boot scripts in series synchronously or asynchronously
// Please note async supports both styles
async.series([
function(done) {
runBootScripts(app, instructions, done);
},
function(done) {
enableAnonymousSwagger(app, instructions);
done();
}], callback);
};

function patchAppLoopback(app) {
Expand Down Expand Up @@ -176,13 +184,36 @@ function forEachKeyedObject(obj, fn) {
});
}

function runScripts(app, list) {
if (!list || !list.length) return;
function runScripts(app, list, callback) {
list = list || [];
var functions = [];
list.forEach(function(filepath) {
debug('Requiring script %s', filepath);
var exports = tryRequire(filepath);
if (typeof exports === 'function')
exports(app);
if (typeof exports === 'function') {
debug('Exported function detected %s', filepath);
functions.push({
path: filepath,
func: exports
});
}
});

async.eachSeries(functions, function(f, done) {
debug('Running script %s', f.path);
if (f.func.length >= 2) {
debug('Starting async function %s', f.path);
f.func(app, function(err) {
debug('Async function finished %s', f.path);
done(err);
});
} else {
debug('Starting sync function %s', f.path);
f.func(app);
debug('Sync function finished %s', f.path);
done();
}
}, callback);
}

function tryRequire(modulePath) {
Expand All @@ -198,8 +229,8 @@ function tryRequire(modulePath) {
}
}

function runBootScripts(app, instructions) {
runScripts(app, instructions.files.boot);
function runBootScripts(app, instructions, callback) {
runScripts(app, instructions.files.boot, callback);
}

function enableAnonymousSwagger(app, instructions) {
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
"url": "https://github.com/strongloop/loopback-boot/blob/master/LICENSE"
},
"dependencies": {
"async": "~0.9.0",
"commondir": "0.0.1",
"debug": "^1.0.4",
"lodash.clonedeep": "^2.4.1",
Expand Down
60 changes: 56 additions & 4 deletions test/executor.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ var path = require('path');
var loopback = require('loopback');
var assert = require('assert');
var expect = require('must');
var fs = require('fs-extra');
var sandbox = require('./helpers/sandbox');
var appdir = require('./helpers/appdir');

Expand Down Expand Up @@ -161,13 +162,62 @@ describe('executor', function() {

describe('with boot and models files', function() {
beforeEach(function() {
process.bootFlags = process.bootFlags || [];
boot.execute(app, simpleAppInstructions());
});

it('should run `boot/*` files', function() {
assert(process.loadedFooJS);
delete process.loadedFooJS;
afterEach(function() {
delete process.bootFlags;
});

it('should run `boot/*` files', function(done) {
// scripts are loaded by the order of file names
expect(process.bootFlags).to.eql([
'barLoaded',
'barSyncLoaded',
'fooLoaded',
'barStarted'
]);

// bar finished happens in the next tick
// barSync executed after bar finished
setTimeout(function() {
expect(process.bootFlags).to.eql([
'barLoaded',
'barSyncLoaded',
'fooLoaded',
'barStarted',
'barFinished',
'barSyncExecuted'
]);
done();
}, 10);
});
});

describe('with boot with callback', function() {
beforeEach(function() {
process.bootFlags = process.bootFlags || [];
});

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If one of these asserts fails, the error message will be very unhelpful.

Solution A:

expect(process.bootFlags.fooLoaded, 'fooLoaded').to.equal(true);
// etc.

Solution B:

expect(process.bootFlags).to.eql({ 
  fooLoaded: true, 
  barLoaded: true, 
  // barProcessed is not set, will be set in the next tick
  barSyncLoaded: true
});

The latter provides most context, as the assert library has all four properties available to print them in the error message.


afterEach(function() {
delete process.bootFlags;
});

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move the hook to the top, right after beforeEach hook.


it('should run `boot/*` files asynchronously', function(done) {
boot.execute(app, simpleAppInstructions(), function() {
expect(process.bootFlags).to.eql([
'barLoaded',
'barSyncLoaded',
'fooLoaded',
'barStarted',
'barFinished',
'barSyncExecuted'
]);
done();
});
});

});

describe('with PaaS and npm env variables', function() {
Expand Down Expand Up @@ -299,5 +349,7 @@ function someInstructions(values) {
}

function simpleAppInstructions() {
return boot.compile(SIMPLE_APP);
// Copy it so that require will happend again
fs.copySync(SIMPLE_APP, appdir.PATH);
return boot.compile(appdir.PATH);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since there is no async app anymore, it would be better to get rid of simpleAsyncAppInstructions altogether. It's not a big deal, I can live with the current code too.

}
8 changes: 8 additions & 0 deletions test/fixtures/simple-app/boot/bar.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
process.bootFlags.push('barLoaded');
module.exports = function(app, callback) {
process.bootFlags.push('barStarted');
process.nextTick(function() {
process.bootFlags.push('barFinished');
callback();
});
};
5 changes: 5 additions & 0 deletions test/fixtures/simple-app/boot/barSync.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
process.bootFlags.push('barSyncLoaded');
module.exports = function(app) {
process.bootFlags.push('barSyncExecuted');
};

2 changes: 1 addition & 1 deletion test/fixtures/simple-app/boot/foo.js
Original file line number Diff line number Diff line change
@@ -1 +1 @@
process.loadedFooJS = true;
process.bootFlags.push('fooLoaded');