Merge pull request #70 from strongloop/feature/short-middleware-paths

#68 - Implement shorthand notation for middleware paths

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 <module>.<fragment> 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;
+}

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);
   });
 }

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'),
+        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));
+      });
   });
 });
 

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('<!DOCTYPE html>\n<html>\n<head lang="en">\n' +
+          '    <meta charset="UTF-8">\n    <title>simple-app</title>\n' +
+          '</head>\n<body>\n<h1>simple-app</h1>\n</body>\n</html>');
+        done();
+      });
+  });
+
   it('configures middleware (end-to-end)', function(done) {
     boot.execute(app, simpleAppInstructions());
 

test/fixtures/simple-app/client/index.html

@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<html>
+<head lang="en">
+    <meta charset="UTF-8">
+    <title>simple-app</title>
+</head>
+<body>
+<h1>simple-app</h1>
+</body>
+</html>

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();
+  };
+};

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();
+};

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"
+}

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);
+};