1
\$\begingroup\$

So in the vein of an old dog trying to learn new tricks, I've decided to try to polish up my JavaScript skills. I've used it a lot but I don't claim expertise. I'm looking for any critique on the approaches I've taken, particularly in style - I can accomplish what I want, but I don't have the intuitive sense of the best approach to take to various problems in this language.

The Goal

My first goal was to come up with a good way to handle routing to an MVC controller in Express. I had looked at some other frameworks (Sails.js, etc.) but wanted to try my own. Basically, I wanted a way I could ask Express to create a route like /:controller/:action and have it route a request such as /user/login to an action called login on a controller called user.

The Design

Some of my design choices:

  1. I wanted it to load all controller files within a given directory. That seemed the right trade-off in terms of granularity. It's important, though, that I could create multiple instances in order to load more than one controller directory.
  2. I defined a 'controller' as any file ending in .js within the given directory. I strip any controller suffix if provided (e.g. either user.js or userController.js defines a controller named user.)
  3. A controller will export 'actions'. An example of two exported actions:

(userController.js):

exports.logout = function(req, res) { /*...*/};

exports.login = {
    get: function(req, res) { /*...*/},
    post: function(req, res) { /*...*/}
}

The above defines an action 'login' with separate handlers for HTTP get and post, and an action 'logout' that handles all HTTP verbs.

An Example of Using It

The module exports one function which takes a directory as input and returns a router suitable to be added to an Express middleware stack. This router defines a method called addRoute. Note that the route must either specify :controller and :action as route segments or these must be defined on an options object. It's perfectly fine to use "/:controller/:action" with both publicRouter and privateRouter - each router will only map those URLs which correspond to actual controllers/actions found and loaded, it doesn't match all two-segment URLs.

var controllerRouter = require("./controller_router");

// Anything in our base ./controllers directory is publicly usable
let publicRouter = controllerRouter(app.get("controllers"));
app.use(publicRouter);

publicRouter.addRoute("/:controller/:action");
publicRouter.addRoute("/:controller", { action: "index" });
publicRouter.addRoute("/login", { controller: "user", action: "login" });
publicRouter.addRoute("/logout", { controller: "user", action: "logout" });


// All routes beyond this point require authenticated users.  
// This middleware redirects unauthenticated users to a login page, etc.
app.use(requireLogin);  

// Load the controllers which require authentication from ./controllers/authenticated
let privateRouter = controllerRouter(path.join(app.get("controllers"), "authenticated"));
app.use(privateRouter);

privateRouter.addRoute("/:controller/:action/:id");
privateRouter.addRoute("/:controller/:action");
privateRouter.addRoute("/:controller", { action: "index" });

The Code Itself

"use strict";

// Modules to load
var express = require("express");
var fs = require("fs");
var path = require("path");


/**
 * Constructor method that builds an object dedicated to routing URLs to actions on controllers contained within a directory.  Call with new.
 * 
 * @param {string} dirname - Directory containing controllers to load
 */
var ControllerRouter = function (dirname) {
    this.controllers = {};
    this.router = express.Router();
    this.loadControllers(dirname);
};

ControllerRouter.prototype = {

    /**
    * Iterates through and loads all files like "name.js" or "nameController.js" in a given directory and maps this.controllers.name to each module.
    *
    * It is expected that each module exports function expressions whose names are the actions provided by that controller.
    *
    * @param {string} dirname - Directory name to search for controllers
    */
    loadControllers: function ControllerRouter_loadControllers(dirname) {
        var files = fs.readdirSync(dirname);

        for (let i = 0; i < files.length; i++) {
            let extension = path.extname(files[i]);
            let filename = path.basename(files[i], extension);

            // Find all .js files except for this one and add them to the list of controllers
            if (extension === ".js" && filename.length > 0) {
                this.controllers[path.basename(filename.toLowerCase(), "controller")] = require(path.join(dirname, filename));
            }
        }
    },


    /**
    * Maps routes to controller actions
    *
    * @param {string} route - A path to map to a controller and action.  May use :controller and :action to map those URL segments
    * @param {Object} options - Controller and/or action to associate with the route, if not included in the path
    */
    addRoute: function ControllerRouter_addRoute(route, options) {
        if (!this.isValidRouteOptions(route, options)) {
            console.log(`Could not map ${route}: Both controller and action must be specified in the route or provided as options`);
            return;
        }

        for (let controller of this.getControllers(options)) {
            for (let action of this.getActions(controller, options)) {
                let routePath = this.replacePath(route, controller, action);
                for (let methodDelegate of this.getMethodsAndDelegates(controller, action)) {
                    this.addRouteHandler(routePath, controller, action, methodDelegate);
                }
            }
        }
    },

    /**
    * Adds a route handler to the router
    *
    * @param {string} routePath  - Route to handle, after any controller/action tokens have been replaced
    * @param {string} controller - Name of the controller to handle this route
    * @param {string} action     - Name of the action to handle this route
    * @param {Object} methodDelegate - Object with method and delegate properties, where method is the HTTP method associated with the delegate
    */
    addRouteHandler: function ControllerRouter_addRouteHandler(routePath, controller, action, methodDelegate) {
        console.log(`${methodDelegate.method.toUpperCase()} ${routePath} => ${controller}.${action}.${methodDelegate.method}`);

        // Here we actually add the route to the router.  We'll wrap the action method to provide a little bit of extra handling
        this.router[methodDelegate.method](routePath, function (req, res, next) {

            // Since :controller and :action weren't really handled as parameters by Express, they wouldn't be in req.params,
            // but we'll be nice and add them in case our controller expects to see them.
            req.params.controller = controller;
            req.params.action = action;

            // Mixin to give our actions an easy way to raise an error - they can call res.raiseError() with an error object.
            res.raiseError = (error) => {
                if (!error || error === "route" ) {  // We won't let any tricksy people try to skip to the next route by raising that string as an error
                    error = new Error(`Unknown error in ${controller}.${action}.${methodDelegate.method}`);
                }
                next(error);
            };

            // Rather than provide next() to our controller's action method, the action is required to either provide a response or raise an error.
            // We'll do an extra bit of error handling by catching any synchronously-thrown exceptions and look for presumably hung responses.

            try {
                // Invoke the handler
                methodDelegate.delegate(req, res);

                // Catch any case where we don't respond fast enough
                let timeout = 10;
                setTimeout(() => {
                    if (!res.headersSent) {
                        let error = new Error(`Action ${controller}.${action}.${methodDelegate.method} failed to return a response or error within ${timeout} seconds`);
                        res.timedOut = true;
                        next(error);
                    }
                }, 1000 * timeout);
            }
            catch (error) {
                next(error);
            }
        });
    },

    /**
    * Iterates through the controllers to be considered.  If options.controllers is provided, and that is a valid controller,
    * only this controller will be returned.  If options.controllers is not provided, all controllers in this.controllers are iterated over.
    *
    * @param {Object} options - Controller and/or action to associate with the route, if not included in the path
    */
    getControllers: function* ControllerRouter_getControllers(options) {
        if (options && typeof options.controller === "string") {
            if (this.controllers.hasOwnProperty(options.controller)) {
                yield options.controller;
            }
        }
        else {
            for (let controller in this.controllers) {
                if (this.controllers.hasOwnProperty(controller)) {
                    yield controller;
                }
            }
        }
    }, 

    /**
    * Iterates through the actions to be considered for a given controller.  If options.action is provided, and that is a valid action on this controller,
    * only this action will be returned.  If options.action is not provided, all actions in the provided controller are iterated over.
    *
    * @param {string} controller - Name of the controller whose actions should be enumerated 
    * @param {Object} options - Controller and/or action to associate with the route, if not included in the path
    */
    getActions: function* ControllerRouter_getActions(controller, options) {
        var c = this.controllers[controller];
        if (options && typeof options.action === "string") {
            if (c.hasOwnProperty(options.action)) {
                yield options.action;
            }
        }
        else {
            for (let action in c) {
                if (c.hasOwnProperty(action)) {
                    yield action;
                }
            }
        }
    }, 

    /**
    * Iterates through the HTTP methods and delegates for a given controller and action.  If a delegate is associated with the method "all" it is always returned last.
    * 
    * @param {string} controller - Name of the controller whose methods should be enumerated
    * @param {string} action - Name of the action whose methods should be enumerated
    */
    getMethodsAndDelegates: function* ControllerRouter_getMethodsAndDelegates(controller, action) {
        if (this.controllers && this.controllers[controller] && this.controllers[controller][action]) {
            var act = this.controllers[controller][action];
            if (typeof act === "function") {
                yield { method: "all", delegate: act };
            }
            else { // Object of methods, always sort "all" to the end
                for (let method in act) {
                    if (act.hasOwnProperty(method) && method !== "all" && typeof act[method] === "function") {
                        yield { method: method, delegate: act[method] };
                    }
                }
                if (act.hasOwnProperty("all") && typeof act.all === "function") {
                    yield { method: "all", delegate: act.all };
                }
            }
        }
    },


    /**
    * Validates that a given route and options are valid.  A route must either specify both the tokens :controller and :action, or the options must specify
    * the missing controller and/or action as properties.
    * 
    * @param {string} route - Route string as passed in to addRoute
    * @param {Object} options - Controller and/or action to associate with the route, if not included in the path
    * @returns {Object} - Yields an object with method and delegate properties, where method is the HTTP method associated with the delegate
    */
    isValidRouteOptions: function ControllerRouter_isValidRouteOptions(route, options) {
        route = route + "/";
        return (route.includes("/:controller/") || options && options.hasOwnProperty("controller") && typeof options.controller === "string" && options.controller.length > 0) &&
            (route.includes("/:action/") || options && options.hasOwnProperty("action") && typeof options.action === "string" && options.action.length > 0);
    },

    /**
    * Takes a string and replaces occurrences of /:controller/ with the actual controller name, and /:action/ with the actual action name
    *
    * @param {string} route - Route string to substitute into
    * @param {string} controller - Name of the controller to be substituted for :controller
    * @param {string} action - Name of the action to be substituted for :action
    * @returns {string} - Route with the controller and action replaced
    */
    replacePath: function ControllerRouter_replacePath(route, controller, action) {
        if (route.slice(-1) !== "/") {
            route = route + "/";
        }

        route = route.split("/:controller/").join("/" + controller + "/").split("/:action/").join("/" + action + "/");
        return route.slice(0, route.length - 1);
    }

};



/**
* This module exports a function that returns a middleware function suitable for inclusion into an Express middleware stack
*
* This middleware function has a .addRoute method which is used to map routes to actions on controllers.
*
* @param {string} dirname - Name of the directory to load controllers from
* @returns {function} - Function suitable for inclusion into a middleware stack
*/
module.exports = function (dirname) {

    var controllerRouter = new ControllerRouter(dirname);

    var handle = (req, res, next) => controllerRouter.router(req, res, next);
    handle.addRoute = (route, options) => controllerRouter.addRoute(route, options);

    return handle;
};

My Main Questions / Thoughts

I debated a lot around what the export of this should be. I was considering exporting either a new ControllerRouter object or the ControllerRouter constructor but instead created a new object and closed over it. My main reason was to give a simple and clean interface. I opted to use a ES5-style constructor function rather than an ES6-style class just to get more familiar with thinking about prototype-based inheritance, not because an ES6 class is a bad approach in my mind.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.