Merge pull request #30 from LabShare/remove-auth

Breaking change: move route authentication out of LabShare/Services

lib/api/loader.js

@@ -1,7 +1,7 @@
 'use strict';
 
 /**
- * @exports A function that can load the API modules located in LabShare packages.
+ * @exports A class that can load the API modules located in LabShare packages.
  *
  * @example
  * var express = require('express');
@@ -22,31 +22,18 @@
  *
  *  Package API module example:
  *  // packages/package-name/src/api/package-service.js
- *  var service = exports;
- *  service.Routes = [ path: '/_api/resource', httpMethod: 'GET', middleware: function (req, res, next) { ... } ];
- *  service.Config = function (data) {
+ *  exports.routes = [ path: '/_api/resource', httpMethod: 'GET', middleware: function (req, res, next) { ... } ];
+ *  exports.config = function (data) {
  *    // custom configuration using the passed in data
  *  };
  */
 
 const path = require('path'),
     assert = require('assert'),
-    validateRoute = require('./validate-route'),
     _ = require('lodash'),
     {Router} = require('express'),
     apiUtils = require('./utils'),
-    ensureAuthorized = require('../auth').EnsureAuthorized;
-
-function normalize(url) {
-    return url
-        .replace(/[\/]+/g, '/')
-        .replace(/\:\//g, '://');
-}
-
-function joinUrl(...args) {
-    var url = args.join('/');
-    return normalize(url);
-}
+    Route = require('./route');
 
 /**
  * @description Retrieves the list of routes from the given module.
@@ -55,24 +42,33 @@ function joinUrl(...args) {
  * @private
  */
 function getRoutes(serviceModule) {
-    if (_.isFunction(serviceModule))  // support revealing module pattern
+    if (_.isFunction(serviceModule)) {  // support revealing module pattern
         serviceModule = serviceModule();
+    }
     return serviceModule.Routes || serviceModule.routes || [];
 }
 
+/**
+ * @description Retrieves the list of routes from the given module.
+ * @param {module} serviceModule - A NodeJS module defining a config function
+ * @returns {Function} - THe module's `config` function
+ * @private
+ */
 function getConfig(serviceModule) {
-    if (_.isFunction(serviceModule))
+    if (_.isFunction(serviceModule)) {
         serviceModule = serviceModule();
+    }
     return serviceModule.Config || serviceModule.config || null;
 }
 
 /**
  * @param {Router} router - An Express JS router
  * @param {Object} route - A route definition
  * @returns {Boolean} true if the route path and method are already stored by the router otherwise false
+ * @private
  */
 function hasRoute(router, route) {
-    return _.some(router.stack, function (item) {
+    return _.some(router.stack, item => {
         return item.route.path === route.path
             && _.has(item.route.methods, route.httpMethod.toLowerCase());
     });
@@ -85,14 +81,12 @@ function hasRoute(router, route) {
  *
  * @param {Object} router An Express JS router that can be extended with new routes and Express JS routers
  * @param {Object} options - Overrides default settings
- *
- * options:
- * {String} options.main - A relative or absolute path to a directory containing a LabShare package. Default: ''
- * {String} options.pattern - The pattern used to match LabShare API modules
- * {Object} options.logger - Error logging provider. It must define an `error` function. Default: null
- * {Array} options.directories - A list of paths to LabShare packages that should be searched for API modules. Directories
+ * @param {String} options.main - A relative or absolute path to a directory containing a LabShare package. Default: ''
+ * @param {String} options.pattern - The pattern used to match LabShare API modules
+ * @param {Object} options.logger - Error logging provider. It must define an `error` function. Default: null
+ * @param {Array} options.directories - A list of paths to LabShare packages that should be searched for API modules. Directories
  * that do not contain a package.json are ignored. Default: [
- * {Array} options.ignore - A list of LabShare package names that should be ignored by the loader. Default: []
+ * @param {Array} options.ignore - A list of LabShare package names that should be ignored by the loader. Default: []
  *
  * @constructor
  */
@@ -101,19 +95,19 @@ class ApiLoader {
         assert.ok(_.isObject(router) && router.use, '`router` must be an express JS router');
 
         this.router = router;
-        this._routers = {};  // format: {packageName: router, ...}
+        this.services = {};  // format: {packageName: [routes], ...}
         this._config = [];   // Stores package API configuration functions
 
         if (options.main) {
-            assert.ok(_.isString(options.main), '`options.main` must be a string');
+            assert.ok(_.isString(options.main), '`main` must be a string');
             options.main = path.resolve(options.main);
         }
         if (options.logger)
-            assert.ok(_.isFunction(options.logger.error), '`options.logger` must define an `error` function');
+            assert.ok(_.isFunction(options.logger.error), '`logger` must define an `error` function');
         if (options.directories) {
             options.directories = apiUtils.wrapInArray(options.directories);
             options.directories = _.map(options.directories, directory => {
-                assert.ok(_.isString(directory), '`options.directories` must contain non-empty strings');
+                assert.ok(_.isString(directory), '`directories` must contain non-empty strings');
                 return path.resolve(directory);
             });
         }
@@ -128,16 +122,15 @@ class ApiLoader {
     }
 
     /**
-     * @description Assigns all the LabShare package REST api routes to the Express JS router
-     * passed to the ApiLoader constructor.
+     * @description Loads all the LabShare package REST api routes
      *
-     * Throws an exception when:
+     * @throws Error when:
      *   - The package directory could not be read
      *   - getServices() fails to load api modules
+     *   - Routes defined by a LabShare package are missing required attributes
      *
      * Logs an error if a logger is defined or throws an exception when:
      *   - A LabShare package's package.json does not contain a name
-     *   - Routes defined by a LabShare package are incorrectly defined
      */
     initialize() {
         try {
@@ -146,7 +139,7 @@ class ApiLoader {
                 apiUtils.applyToNodeModulesSync(this.options.main, this._loadRoutes.bind(this));
             }
         } catch (error) {
-            error.message = 'Failed to load routes: ' + error.message;
+            error.message = `Failed to load routes: ${error.message}`;
             this._handleError(error);
         }
     };
@@ -156,13 +149,17 @@ class ApiLoader {
      * @param {string} [mountPoint] - The base route where each route is accessible. For example: '/_api'.
      * @api
      */
-    setAPIs(mountPoint) {
-        mountPoint = mountPoint || '/';
+    setAPIs(mountPoint = '/') {
+        assert.ok(_.isString(mountPoint), 'ApiLoader.setAPIs: `mountPoint` must be a string!');
 
-        assert.ok(mountPoint && _.isString(mountPoint), 'ApiLoader.setAPIs: `mountPoint` must be a string!');
+        _.each(this.services, (routes, packageName) => {
+            let router = Router({mergeParams: true});
 
-        _.each(this._routers, (packageRouter) => {
-            this.router.use(mountPoint, packageRouter);
+            _.each(routes, route => {
+                this._assignRoute(route, router, packageName);
+            });
+
+            this.router.use(mountPoint, router);
         });
     };
 
@@ -183,30 +180,24 @@ class ApiLoader {
 
     /**
      * @description Locates all the API modules from the given directory
-     * then assigns them to the Express JS router.
+     * then caches them
      * @param {String} directory An absolute path to a directory
      * @private
      */
     _loadRoutes(directory) {
-        var manifest = apiUtils.getPackageManifest(directory);
+        let manifest = apiUtils.getPackageManifest(directory);
         if (!manifest || apiUtils.isIgnored(manifest, this.options.ignore)) {
             return;
         }
 
-        var packageName = apiUtils.getPackageName(manifest),
-            router = this._routers[packageName];
+        let packageName = apiUtils.getPackageName(manifest);
 
-        if (router) {
-            return;  // avoid assigning duplicate routers to this.router
-        }
+        this.services[packageName] = this.services[packageName] || [];
 
-        router = Router({mergeParams: true});
-        this._routers[packageName] = router;
+        let serviceModulePaths = apiUtils.getMatchingFilesSync(directory, this.options.pattern);
 
-        var serviceModulePaths = apiUtils.getMatchingFilesSync(directory, this.options.pattern);
-
-        _.each(serviceModulePaths, (serviceModulePath) => {
-            var module = require(serviceModulePath),
+        _.each(serviceModulePaths, serviceModulePath => {
+            let module = require(serviceModulePath),
                 routes = getRoutes(module),
                 configFunction = getConfig(module);
 
@@ -218,8 +209,13 @@ class ApiLoader {
                 }
             }
 
-            _.each(routes, (route) => {
-                this._assignRoute(route, router, packageName);
+            _.each(routes, routeOptions => {
+                try {
+                    let route = new Route(packageName, routeOptions);
+                    this.services[packageName].push(route);
+                } catch (error) {
+                    this._handleError(error);
+                }
             });
         });
     };
@@ -233,40 +229,17 @@ class ApiLoader {
      * @private
      */
     _assignRoute(route, router, packageName) {
-        let routeValidation = validateRoute(route, packageName);
-        if (!routeValidation.isValid) {
-            this._handleError(new Error(routeValidation.message));
-            return;
-        }
-
         if (hasRoute(router, route)) {
             return;
         }
 
-        this._checkIfDuplicate(route, packageName);
-
-        route.middleware = apiUtils.wrapInArray(route.middleware);
-        route.middleware.unshift(ensureAuthorized(route));  // attach authorization middleware to the route
-
-        // Namespace the route paths
-        let args = _.flatten([joinUrl('/', packageName, route.path), route.middleware]);
+        let args = _.flatten([route.path, route.middleware]),
+            httpMethod = (route.httpMethod || '').toLowerCase();
 
-        switch (route.httpMethod.toUpperCase()) {
-            case 'GET':
-                router.get(...args);
-                break;
-            case 'POST':
-                router.post(...args);
-                break;
-            case 'PUT':
-                router.put(...args);
-                break;
-            case 'DELETE':
-                router.delete(...args);
-                break;
-            default:
-                this._handleError(new Error(`Invalid HTTP method specified for route ${route.path} from package ${packageName}`));
-                break;
+        if (!_.isFunction(router[httpMethod])) {
+            this._handleError(new Error(`Invalid HTTP method specified for route ${route.path} from package ${packageName}`));
+        } else {
+            router[httpMethod](...args);
         }
     };
 
@@ -277,20 +250,6 @@ class ApiLoader {
         }
         throw error;
     };
-
-    /**
-     * @description Checks if the route has already been assigned by another package
-     * @param {object} route - A route definition
-     * @param {string} packageName
-     * @private
-     */
-    _checkIfDuplicate(route, packageName) {
-        _.each(this._routers, (router, name) => {
-            if (hasRoute(router, route) && name !== packageName) {
-                this._handleError(new Error(`Duplicate route: ${route.httpMethod} ${route.path} from "${packageName}" was already loaded by "${name}"`));
-            }
-        });
-    };
 }
 
 module.exports = ApiLoader;

lib/api/route.js

@@ -0,0 +1,48 @@
+'use strict';
+
+const validateRoute = require('./validate-route'),
+    _ = require('lodash');
+
+function normalize(url) {
+    return url
+        .replace(/[\/]+/g, '/')
+        .replace(/\:\//g, '://');
+}
+
+function joinUrl(...args) {
+    var url = args.join('/');
+    return normalize(url);
+}
+
+function toArray(arg) {
+    return !_.isArray(arg) ? [arg] : arg;
+}
+
+class Route {
+
+    /**
+     * @description Creates a new route instance
+     * @param {String} packageName - The name of the LabShare package that created the API route
+     * @param {Object} options
+     * @param {String} options.path - The API endpoint (e.g. '/users/create')
+     * @param {String} options.httpMethod - The HTTP verb
+     * @param {Array|Function} options.middleware - One or more Express.JS middleware functions
+     */
+    constructor(packageName, options = {}) {
+        let validator = validateRoute(options, packageName);
+        if (!validator.isValid) {
+            throw new Error(validator.message);
+        }
+
+        _.extend(this, options);
+
+        this.middleware = toArray(this.middleware);
+
+        // Namespace the API path with the LabShare package name
+        if (!this.path.startsWith(`/${packageName}`)) {
+            this.path = joinUrl('/', packageName, this.path);
+        }
+    }
+}
+
+module.exports = Route;

lib/api/utils.js

@@ -16,7 +16,7 @@ const glob = require('glob'),
  * @returns {Object} Containing LabShare package dependencies or an empty object
  */
 exports.getPackageDependencies = function getPackageDependencies(manifest) {
-    return (_.isObject(manifest) && _.isObject(manifest.packageDependencies))
+    return (_.isObject(manifest) && _.isPlainObject(manifest.packageDependencies))
         ? manifest.packageDependencies
         : {};
 };

lib/api/validate-route.js

@@ -23,7 +23,6 @@ module.exports = function validateRoute(route, packageName) {
                     allowEmpty: false
                 },
                 middleware: {
-                    // type: ['array', 'object'],
                     conform: middleware => {
                         return _.isFunction(middleware) || _.isArray(middleware);
                     },
@@ -38,8 +37,6 @@ module.exports = function validateRoute(route, packageName) {
                     allowEmpty: false
                 }
             }
-        }, {
-            additionalProperties: false
         }
     );