diff --git a/lib/compiler.js b/lib/compiler.js index 2f6427b..6cfd3ad 100644 --- a/lib/compiler.js +++ b/lib/compiler.js @@ -354,23 +354,24 @@ function buildMiddlewareInstructions(rootDir, config) { phasesNames.forEach(function(phase) { var phaseConfig = config[phase]; Object.keys(phaseConfig).forEach(function(middleware) { - var start = middleware.substring(0, 2); - var sourceFile = start !== './' && start !== '..' ? - middleware : - path.resolve(rootDir, middleware); - var allConfigs = phaseConfig[middleware]; if (!Array.isArray(allConfigs)) allConfigs = [allConfigs]; allConfigs.forEach(function(config) { + var resolved = resolveMiddlewarePath(rootDir, middleware); + var middlewareConfig = cloneDeep(config); middlewareConfig.phase = phase; - middlewareList.push({ - sourceFile: require.resolve(sourceFile), + var item = { + sourceFile: resolved.sourceFile, config: middlewareConfig - }); + }; + if (resolved.fragment) { + item.fragment = resolved.fragment; + } + middlewareList.push(item); }); }); }); @@ -390,3 +391,60 @@ function buildMiddlewareInstructions(rootDir, config) { middleware: middlewareList }; } + +function resolveMiddlewarePath(rootDir, middleware) { + var resolved = {}; + + var segments = middleware.split('#'); + var pathName = segments[0]; + var fragment = segments[1]; + + if (fragment) { + resolved.fragment = fragment; + } + + if (pathName.indexOf('./') === 0 || pathName.indexOf('../') === 0) { + // Relative path + pathName = path.resolve(rootDir, pathName); + } + + if (!fragment) { + resolved.sourceFile = require.resolve(pathName); + return resolved; + } + + var err; + + // Try to require the module and check if . is a valid + // function + var m = require(pathName); + if (typeof m[fragment] === 'function') { + resolved.sourceFile = require.resolve(pathName); + return resolved; + } + + /* + * module/server/middleware/fragment + * module/middleware/fragment + */ + var candidates = [ + pathName + '/server/middleware/' + fragment, + pathName + '/middleware/' + fragment, + // TODO: [rfeng] Should we support the following flavors? + // pathName + '/lib/' + fragment, + // pathName + '/' + fragment + ]; + + for (var ix in candidates) { + try { + resolved.sourceFile = require.resolve(candidates[ix]); + delete resolved.fragment; + return resolved; + } + catch (e) { + // Report the error for the first candidate when no candidate matches + if (!err) err = e; + } + } + throw err; +} diff --git a/lib/executor.js b/lib/executor.js index 8218639..c8e3865 100644 --- a/lib/executor.js +++ b/lib/executor.js @@ -257,7 +257,8 @@ function setupMiddleware(app, instructions) { return; } - var phases = instructions.middleware.phases; + // Phases can be empty + var phases = instructions.middleware.phases || []; assert(Array.isArray(phases), 'instructions.middleware.phases must be an array'); @@ -269,8 +270,14 @@ function setupMiddleware(app, instructions) { app.defineMiddlewarePhases(phases); middleware.forEach(function(data) { - debug('Configuring middleware %j', data.sourceFile); + debug('Configuring middleware %j%s', data.sourceFile, + data.fragment ? ('#' + data.fragment) : ''); var factory = require(data.sourceFile); + if (data.fragment) { + factory = factory[data.fragment]; + } + assert(typeof factory === 'function', + 'Middleware factory must be a function'); app.middlewareFromConfig(factory, data.config); }); } diff --git a/test/compiler.test.js b/test/compiler.test.js index 331c3d4..a4f9a1d 100644 --- a/test/compiler.test.js +++ b/test/compiler.test.js @@ -675,34 +675,52 @@ describe('compiler', function() { }); describe('for middleware', function() { - beforeEach(function() { - appdir.createConfigFilesSync(); - }); - it('emits middleware instructions', function() { - appdir.writeConfigFileSync('middleware.json', { + function testMiddlewareRegistration(middlewareId, sourceFile) { + var json = { initial: { }, custom: { - 'loopback/server/middleware/url-not-found': { - params: 'some-config-data' - } - }, - }); + } + }; + + json.custom[middlewareId] = { + params: 'some-config-data' + }; + + appdir.writeConfigFileSync('middleware.json', json); var instructions = boot.compile(appdir.PATH); expect(instructions.middleware).to.eql({ phases: ['initial', 'custom'], - middleware: [{ - sourceFile: - require.resolve('loopback/server/middleware/url-not-found'), - config: { - phase: 'custom', - params: 'some-config-data' + middleware: [ + { + sourceFile: sourceFile, + config: { + phase: 'custom', + params: 'some-config-data' + } } - }] + ] }); + } + + var sourceFileForUrlNotFound; + beforeEach(function() { + fs.copySync(SIMPLE_APP, appdir.PATH); + sourceFileForUrlNotFound = require.resolve( + 'loopback/server/middleware/url-not-found'); + }); + + it('emits middleware instructions', function() { + testMiddlewareRegistration('loopback/server/middleware/url-not-found', + sourceFileForUrlNotFound); + }); + + it('emits middleware instructions for fragment', function() { + testMiddlewareRegistration('loopback#url-not-found', + sourceFileForUrlNotFound); }); it('fails when a module middleware cannot be resolved', function() { @@ -716,6 +734,20 @@ describe('compiler', function() { .to.throw(/path-does-not-exist/); }); + it('fails when a module middleware fragment cannot be resolved', + function() { + appdir.writeConfigFileSync('middleware.json', { + final: { + 'loopback#path-does-not-exist': { } + } + }); + + expect(function() { + boot.compile(appdir.PATH); + }) + .to.throw(/path-does-not-exist/); + }); + it('resolves paths relatively to appRootDir', function() { appdir.writeConfigFileSync('./middleware.json', { routes: { @@ -750,7 +782,7 @@ describe('compiler', function() { routes: { './middleware': { params: { - key: 'custom value', + key: 'custom value' } } } @@ -829,7 +861,7 @@ describe('compiler', function() { params: 'second' } ] - }, + } }); var instructions = boot.compile(appdir.PATH); @@ -849,9 +881,79 @@ describe('compiler', function() { phase: 'final', params: 'second' } - }, + } ]); }); + + it('supports shorthand notation for middleware paths', function() { + appdir.writeConfigFileSync('middleware.json', { + 'final': { + 'loopback#url-not-found': {} + } + }); + + var instructions = boot.compile(appdir.PATH); + + expect(instructions.middleware.middleware[0].sourceFile) + .to.equal(require.resolve('loopback/server/middleware/url-not-found')); + }); + + it('supports shorthand notation for relative paths', function() { + appdir.writeConfigFileSync('middleware.json', { + 'routes': { + './middleware/index#myMiddleware': { + } + } + }); + + var instructions = boot.compile(appdir.PATH); + + expect(instructions.middleware.middleware[0].sourceFile) + .to.equal(path.resolve(appdir.PATH, + './middleware/index.js')); + expect(instructions.middleware.middleware[0]).have.property( + 'fragment', + 'myMiddleware'); + }); + + it('supports shorthand notation when the fragment name matches a property', + function() { + appdir.writeConfigFileSync('middleware.json', { + 'final': { + 'loopback#errorHandler': {} + } + }); + + var instructions = boot.compile(appdir.PATH); + + expect(instructions.middleware.middleware[0]).have.property( + 'sourceFile', + require.resolve('loopback')); + expect(instructions.middleware.middleware[0]).have.property( + 'fragment', + 'errorHandler'); + }); + + // FIXME: [rfeng] The following test is disabled until + // https://github.com/strongloop/loopback-boot/issues/73 is fixed + it.skip('resolves modules relative to appRootDir', function() { + var HANDLER_FILE = 'node_modules/handler/index.js'; + appdir.writeFileSync( + HANDLER_FILE, + 'module.exports = function(req, res, next) { next(); }'); + + appdir.writeConfigFileSync('middleware.json', { + 'initial': { + 'handler': {} + } + }); + + var instructions = boot.compile(appdir.PATH); + + expect(instructions.middleware.middleware[0]).have.property( + 'sourceFile', + appdir.resolve(HANDLER_FILE)); + }); }); }); diff --git a/test/executor.test.js b/test/executor.test.js index e4ffd92..c6fd891 100644 --- a/test/executor.test.js +++ b/test/executor.test.js @@ -375,6 +375,34 @@ describe('executor', function() { }); }); + it('configures middleware using shortform', function(done) { + + boot.execute(app, someInstructions({ + middleware: { + middleware: [ + { + sourceFile: require.resolve('loopback'), + fragment: 'static', + config: { + phase: 'files', + params: path.join(__dirname, './fixtures/simple-app/client/') + } + } + ] + } + })); + + supertest(app) + .get('/') + .end(function(err, res) { + if (err) return done(err); + expect(res.text).to.eql('\n\n\n' + + ' \n simple-app\n' + + '\n\n

simple-app

\n\n'); + done(); + }); + }); + it('configures middleware (end-to-end)', function(done) { boot.execute(app, simpleAppInstructions()); diff --git a/test/fixtures/simple-app/client/index.html b/test/fixtures/simple-app/client/index.html new file mode 100644 index 0000000..df535e6 --- /dev/null +++ b/test/fixtures/simple-app/client/index.html @@ -0,0 +1,10 @@ + + + + + simple-app + + +

simple-app

+ + \ No newline at end of file diff --git a/test/fixtures/simple-app/middleware/index.js b/test/fixtures/simple-app/middleware/index.js new file mode 100644 index 0000000..d2b20d9 --- /dev/null +++ b/test/fixtures/simple-app/middleware/index.js @@ -0,0 +1,8 @@ +exports.myMiddleware = function(name) { + return function(req, res, next) { + req._names = req._names || []; + req._names.push(name); + res.setHeader('names', req._names.join(',')); + next(); + }; +}; diff --git a/test/fixtures/simple-app/node_modules/my-module/index.js b/test/fixtures/simple-app/node_modules/my-module/index.js new file mode 100644 index 0000000..5b64f75 --- /dev/null +++ b/test/fixtures/simple-app/node_modules/my-module/index.js @@ -0,0 +1,7 @@ +/** + * Exporting a middleware as a property of the main module + */ +exports.myMiddleware = function(req, res, next) { + res.setHeader('X-MY-MIDDLEWARE', 'myMiddleware'); + next(); +}; diff --git a/test/fixtures/simple-app/node_modules/my-module/package.json b/test/fixtures/simple-app/node_modules/my-module/package.json new file mode 100644 index 0000000..07f8627 --- /dev/null +++ b/test/fixtures/simple-app/node_modules/my-module/package.json @@ -0,0 +1,7 @@ +{ + "name": "my-module", + "version": "1.0.0", + "description": "my-module", + "main": "index.js", + "license": "MIT" +} diff --git a/test/helpers/appdir.js b/test/helpers/appdir.js index 1a31be5..a52a606 100644 --- a/test/helpers/appdir.js +++ b/test/helpers/appdir.js @@ -42,8 +42,12 @@ appdir.writeConfigFileSync = function(name, json) { }; appdir.writeFileSync = function(name, content) { - var filePath = path.resolve(PATH, name); + var filePath = this.resolve(name); fs.mkdirsSync(path.dirname(filePath)); fs.writeFileSync(filePath, content, 'utf-8'); return filePath; }; + +appdir.resolve = function(name) { + return path.resolve(PATH, name); +};