Merge pull request #113 from PradnyaBaviskar/lb-boot-issue-73-4

Resolve module paths as relative to appRootDir - for middleware

lib/compiler.js

@@ -271,7 +271,7 @@ function findModelDefinitions(rootDir, sources) {
   var registry = {};
 
   sources.forEach(function(src) {
-    var srcDir = resolveAppPath(rootDir, src);
+    var srcDir = tryResolveAppPath(rootDir, src);
     if (!srcDir) {
       debug('Skipping unknown module source dir %j', src);
       return;
@@ -300,6 +300,16 @@ function findModelDefinitions(rootDir, sources) {
 }
 
 function resolveAppPath(rootDir, relativePath) {
+  var resolvedPath = tryResolveAppPath(rootDir, relativePath);
+  if (resolvedPath === undefined) {
+    var err = new Error('Cannot resolve path "' + relativePath + '"');
+    err.code = 'PATH_NOT_FOUND';
+    throw err;
+  }
+  return resolvedPath;
+}
+
+function tryResolveAppPath(rootDir, relativePath) {
   var fullPath = path.resolve(rootDir, relativePath);
   if (fs.existsSync(fullPath))
     return fullPath;
@@ -434,6 +444,7 @@ function resolveMiddlewarePath(rootDir, middleware) {
   var segments = middleware.split('#');
   var pathName = segments[0];
   var fragment = segments[1];
+  var middlewarePath = pathName;
 
   if (fragment) {
     resolved.fragment = fragment;
@@ -445,7 +456,7 @@ function resolveMiddlewarePath(rootDir, middleware) {
   }
 
   if (!fragment) {
-    resolved.sourceFile = require.resolve(pathName);
+    resolved.sourceFile = resolveAppPath(rootDir, middlewarePath);
     return resolved;
   }
 
@@ -455,7 +466,7 @@ function resolveMiddlewarePath(rootDir, middleware) {
   // function
   var m = require(pathName);
   if (typeof m[fragment] === 'function') {
-    resolved.sourceFile = require.resolve(pathName);
+    resolved.sourceFile = resolveAppPath(rootDir, middlewarePath);
     return resolved;
   }
 
@@ -473,7 +484,7 @@ function resolveMiddlewarePath(rootDir, middleware) {
 
   for (var ix in candidates) {
     try {
-      resolved.sourceFile = require.resolve(candidates[ix]);
+      resolved.sourceFile = resolveAppPath(rootDir, candidates[ix]);
       delete resolved.fragment;
       return resolved;
     }
@@ -519,7 +530,7 @@ function buildComponentInstructions(rootDir, componentConfig) {
 
 function resolveRelativePaths(relativePaths, appRootDir) {
   relativePaths.forEach(function(relativePath, k) {
-    var resolvedPath = resolveAppPath(appRootDir, relativePath);
+    var resolvedPath = tryResolveAppPath(appRootDir, relativePath);
     if (resolvedPath !== undefined) {
       relativePaths[k] = resolvedPath;
     } else {

test/compiler.test.js

@@ -858,10 +858,11 @@ describe('compiler', function() {
       });
 
     it('resolves paths relatively to appRootDir', function() {
+      appdir.writeFileSync('my-middleware.js', '');
       appdir.writeConfigFileSync('./middleware.json', {
         routes: {
-          // resolves to ./middleware.json
-          './middleware': { }
+          // resolves to ./my-middleware.js
+          './my-middleware': { }
         }
       });
 
@@ -870,7 +871,7 @@ describe('compiler', function() {
       expect(instructions.middleware).to.eql({
         phases: ['routes'],
         middleware: [{
-          sourceFile: path.resolve(appdir.PATH, 'middleware.json'),
+          sourceFile: path.resolve(appdir.PATH, 'my-middleware.js'),
           config: { phase: 'routes' }
         }]
       });
@@ -959,10 +960,10 @@ describe('compiler', function() {
     });
 
     it('supports multiple instances of the same middleware', function() {
-
+      appdir.writeFileSync('my-middleware.js', '');
       appdir.writeConfigFileSync('middleware.json', {
         'final': {
-          './middleware': [
+          './my-middleware': [
             {
               params: 'first'
             },
@@ -978,14 +979,14 @@ describe('compiler', function() {
       expect(instructions.middleware.middleware)
         .to.eql([
           {
-            sourceFile: path.resolve(appdir.PATH, 'middleware.json'),
+            sourceFile: path.resolve(appdir.PATH, 'my-middleware.js'),
             config: {
               phase: 'final',
               params: 'first'
             }
           },
           {
-            sourceFile: path.resolve(appdir.PATH, 'middleware.json'),
+            sourceFile: path.resolve(appdir.PATH, 'my-middleware.js'),
             config: {
               phase: 'final',
               params: 'second'
@@ -1043,27 +1044,58 @@ describe('compiler', function() {
           '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(); }');
+    it('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));
+      appdir.writeConfigFileSync('middleware.json', {
+        'initial': {
+          'handler': {}
+        }
       });
 
+      var instructions = boot.compile(appdir.PATH);
+
+      expect(instructions.middleware.middleware[0]).have.property(
+        'sourceFile',
+        appdir.resolve(HANDLER_FILE));
+    });
+
+    it('prefers appRootDir over node_modules for middleware', function() {
+      var appJS = appdir.writeFileSync('./my-middleware.js', '');
+      appdir.writeFileSync('node_modules/my-middleware.js', '');
+      appdir.writeConfigFileSync('middleware.json', {
+        'routes': {
+          './my-middleware': {}
+        }
+      });
+
+      var instructions = boot.compile(appdir.PATH);
+
+      expect(instructions.middleware.middleware).to.have.length(1);
+      expect(instructions.middleware.middleware[0]).have.property(
+        'sourceFile', appJS);
+    });
+
+    it('does not treat module relative path as `appRootDir` relative',
+      function() {
+      appdir.writeFileSync('./my-middleware.js', '');
+      var moduleJS = appdir.writeFileSync('node_modules/my-middleware.js', '');
+      appdir.writeConfigFileSync('middleware.json', {
+        'routes': {
+          'my-middleware': {}
+        }
+      });
+
+      var instructions = boot.compile(appdir.PATH);
+
+      expect(instructions.middleware.middleware).to.have.length(1);
+      expect(instructions.middleware.middleware[0]).have.property(
+        'sourceFile', moduleJS);
+    });
+
     describe('config with relative paths in params', function() {
       var RELATIVE_PATH_PARAMS = [
         '$!./here',