Do not require both expressApp and router (#177)

CHANGELOG.md

@@ -1,6 +1,7 @@
 ## Changelog
 
 ### 4.0.0 (Next)
+* [#176](https://github.com/alexa-js/alexa-app/pull/176): Express integration now requires either `expressApp` or `router`, but not both - [@bgjehu](https://github.com/bgjehu).
 * [#174](https://github.com/alexa-js/alexa-app/pull/174): Always enfore strict header checking - [@tejashah88](https://github.com/tejashah88).
 * Your contribution here.
 

README.md

@@ -79,7 +79,7 @@ app.intent("number", {
 );
 
 // setup the alexa app and attach it to express before anything else
-app.express({ expressApp: express_app, router: express.Router() });
+app.express({ expressApp: express_app });
 
 // now POST calls to /sample in express will be handled by the app.request() function
 // GET calls will not be handled
@@ -89,14 +89,15 @@ app.express({ expressApp: express_app, router: express.Router() });
 
 The express function accepts the following parameters.
 
-* `expressApp` the express instance to attach to
-* `router` router instance to attach to the express app
-* `endpoint` the path to attach the router to (e.g., passing `'mine'` attaches to `/mine`)
+* `expressApp` the express app instance to attach to
+* `router` the express router instance to attach to
+* `endpoint` the path to attach the express app or router to (e.g., passing `'mine'` attaches to `/mine`)
 * `checkCert` when true, applies Alexa certificate checking _(default: true)_
 * `debug` when true, sets up the route to handle GET requests _(default: false)_
 * `preRequest` function to execute before every POST
 * `postRequest` function to execute after every POST
 
+Either `expressApp` or `router` is required.
 A full express example is available [here](example/express.js).
 
 

UPGRADING.md

@@ -39,6 +39,48 @@ app.intent("tellme", (request, response) => {
 });
 ```
 
+#### Changes to ExpressJS Support
+
+Express.js integration via `app.express()` now requires one of `expresApp` or `router`. When using a router, it will no longer be automatically mounted inside the express app and you may need to implement that outside of this call.
+
+##### Use default endpoint
+
+before:
+```javascript
+var express_app = express();
+var app = new alexa.app("sample");
+app.express({ expressApp: express_app, router: express.Router() });
+// now POST calls to /sample in express will be handled by the app.request() function
+```
+
+after:
+```javascript
+var express_app = express();
+var app = new alexa.app("sample");
+app.express({ expressApp: express_app });
+// now POST calls to /sample in express will be handled by the app.request() function
+```
+
+##### Use specified endpoint
+
+before:
+```javascript
+var express_app = express();
+var app = new alexa.app("sample");
+app.express({ expressApp: express_app, router: express.Router(), endpoint: 'api/alexa' });
+// now POST calls to /api/alexa in express will be handled by the app.request() function
+```
+
+after:
+```javascript
+var express_app = express();
+var apiRouter = express.Router();
+var app = new alexa.app("sample");
+app.express({ router: apiRouter, endpoint: '/alexa' });
+express_app.use('/api', apiRouter);
+// now POST calls to /api/alexa in express will be handled by the app.request() function
+```
+
 ### Upgrading to >= 3.0.0
 
 #### Changes in Express integration interface

example/express.js

@@ -9,7 +9,6 @@ var alexaApp = new alexa.app("test");
 
 alexaApp.express({
   expressApp: app,
-  router: express.Router(),
 
   // verifies requests come from amazon alexa. Must be enabled for production.
   // You can disable this if you're running a dev environment and want to POST

index.js

@@ -7,6 +7,7 @@ var alexa = {};
 var defaults = require("lodash.defaults");
 var verifier = require("alexa-verifier-middleware");
 var bodyParser = require('body-parser');
+var utils = require('./utils');
 
 alexa.response = function(session) {
   var self = this;
@@ -560,25 +561,26 @@ alexa.app = function(name) {
   // @throws Error when router or expressApp options are not specified
   // @returns this
   this.express = function(options) {
-    if (!options.expressApp) {
-      throw new Error("You must specify an express instance to attach to.");
+    if (!options.expressApp && !options.router) {
+      throw new Error("You must specify an express app or an express router to attach to.");
     }
 
-    if (!options.router) {
-      throw new Error("You must specify an express router to attach.");
-    }
-
-    var defaultOptions = { endpoint: self.name, checkCert: true, debug: false };
+    var defaultOptions = { endpoint: "/" + self.name, checkCert: true, debug: false };
 
     options = defaults(options, defaultOptions);
 
-    var endpoint = "/" + options.endpoint;
-    var router = options.router;
+    // In ExpressJS, user specifies their paths without the '/' prefix
+    var deprecated = options.expressApp && options.router;
+    var endpoint = deprecated ? '/' : utils.normalizeApiPath(options.endpoint);
+    var target = deprecated ? options.router : (options.expressApp || options.router);
 
-    options.expressApp.use(endpoint, router);
+    if (deprecated) {
+      options.expressApp.use(utils.normalizeApiPath(options.endpoint), options.router);
+      console.warn("Usage deprecated: Both 'expressApp' and 'router' are specified.\nMore details on https://github.com/alexa-js/alexa-app/blob/master/UPGRADING.md");
+    }
 
     if (options.debug) {
-      options.router.get("/", function(req, res) {
+      target.get(endpoint, function(req, res) {
         if (typeof req.query['schema'] != "undefined") {
           res.set('Content-Type', 'text/plain').send(self.schema());
         } else if (typeof req.query['utterances'] != "undefined") {
@@ -594,13 +596,13 @@ alexa.app = function(name) {
     }
 
     if (options.checkCert) {
-      options.router.use(verifier);
+      target.use(endpoint, verifier);
     } else {
-      options.router.use(bodyParser.json());
+      target.use(endpoint, bodyParser.json());
     }
 
     // exposes POST /<endpoint> route
-    router.post("/", function(req, res) {
+    target.post(endpoint, function(req, res) {
       var json = req.body,
         response_json;
       // preRequest and postRequest may return altered request JSON, or undefined, or a Promise

package.json

@@ -17,7 +17,7 @@
     "lint": "eslint index.js;",
     "coverage": "istanbul cover _mocha -- -R spec",
     "test": "./node_modules/.bin/mocha",
-    "test-with-coverage": "./node_modules/istanbul/lib/cli.js cover ./node_modules/mocha/bin/_mocha -- -R spec ./test/*",
+    "test-with-coverage": "./node_modules/istanbul/lib/cli.js cover ./node_modules/mocha/bin/_mocha -- -R spec ./test/*.js",
     "danger": "./node_modules/.bin/danger"
   },
   "repository": {

test/example.json

@@ -0,0 +1,3 @@
+{
+  "pangram": "The quick brown fox jumps over the lazy dog"
+}

test/example.txt

@@ -0,0 +1 @@
+The quick brown fox jumps over the lazy dog

test/test_alexa_integration_express.js

@@ -37,27 +37,29 @@ describe("Alexa", function() {
     });
 
     context("#express fails when missing required field", function() {
-      it("throws error on missing express app", function() {
+      it("throws error on missing express app and express router", function() {
         try {
-          testApp.express({ router: express.Router() });
+          testApp.express({});
         } catch (er) {
-          return expect(er.message).to.eq("You must specify an express instance to attach to.")
-        }
-      });
-
-      it("throws error on missing express router", function() {
-        try {
-          testApp.express({ expressApp: app });
-        } catch (er) {
-          return expect(er.message).to.eq("You must specify an express router to attach.")
+          return expect(er.message).to.eq("You must specify an express app or an express router to attach to.")
         }
       });
+    });
 
+    context("#express warns when redundant param is passed", function() {
+        it("warns on given both params 'expressApp' and 'router'", function() {
+          var bkp = console.warn.bind();
+          console.warn = sinon.spy();
+          testApp.express({expressApp: app, router: express.Router()});
+          var warning = "Usage deprecated: Both 'expressApp' and 'router' are specified.\nMore details on https://github.com/alexa-js/alexa-app/blob/master/UPGRADING.md";
+          expect(console.warn).to.have.been.calledWithExactly(warning);
+          console.warn = bkp;
+        });
     });
 
     context("#express with default options", function() {
       beforeEach(function() {
-        testApp.express({ expressApp: app, router: express.Router(), checkCert: false });
+        testApp.express({ expressApp: app, checkCert: false });
       });
 
       it("returns a response for a valid request", function() {
@@ -134,7 +136,9 @@ describe("Alexa", function() {
 
     context("#express with debug set to false", function() {
       beforeEach(function() {
-        testApp.express({ expressApp: app, router: express.Router(), checkCert: false, debug: false });
+        var router = express.Router();
+        testApp.express({ router: router, checkCert: false, debug: false });
+        app.use(router);
       });
 
       it("cannot dump debug schema", function() {

test/test_utils.js

@@ -0,0 +1,83 @@
+"use strict";
+var chai = require("chai");
+var expect = chai.expect;
+chai.config.includeStack = true;
+var utils = require("../utils");
+
+describe("Utils", function() {
+  describe("#isValidFile", function() {
+    it("verifies that 'README.md' does exists and is a file", function() {
+      expect(utils.isValidFile('README.md')).to.be.true;
+    });
+
+    it("verifies that 'RANDOM.md' does not exists", function() {
+      expect(utils.isValidFile('RANDOM.md')).to.be.false;
+    });
+
+    it("verifies that 'examples' is not a file", function() {
+      expect(utils.isValidFile('examples')).to.be.false;
+    });
+  });
+
+  describe("#isValidDirectory", function() {
+    it("verifies that 'example' does exists and is a directory", function() {
+      expect(utils.isValidDirectory('example')).to.be.true;
+    });
+
+    it("verifies that 'random' does not exists", function() {
+      expect(utils.isValidDirectory('random')).to.be.false;
+    });
+
+    it("verifies that 'README.md' is not a directory", function() {
+      expect(utils.isValidDirectory('README.md')).to.be.false;
+    });
+  });
+
+  describe("#readFile", function() {
+    it("successfully reads 'example.txt'", function() {
+      expect(utils.readFile('test/example.txt')).to.equal("The quick brown fox jumps over the lazy dog");
+    });
+
+    it("throws an error when trying to read 'example.text'", function() {
+      expect(function() {
+        utils.readFile('test/example.text');
+      }).to.throw(Error);
+    });
+  });
+
+  describe("#readJsonFile", function() {
+    it("successfully reads 'example.json'", function() {
+      expect(utils.readJsonFile('test/example.json')).to.deep.equal({
+        "pangram": "The quick brown fox jumps over the lazy dog"
+      });
+    });
+
+    it("throws an error when trying to read 'example.jason'", function() {
+      expect(function() {
+        utils.readJsonFile('test/example.jason');
+      }).to.throw(Error);
+    });
+  });
+
+  describe("#normalizeApiPath", function() {
+    var tests = [
+      { original: "alexa",       final: "/alexa" },
+      { original: "/alexa",      final: "/alexa" },
+      { original: "//alexa",     final: "/alexa" },
+      { original: "///alexa",    final: "/alexa" },
+      { original: "alexa/",      final: "/alexa/" },
+      { original: "alexa//",     final: "/alexa/" },
+      { original: "alexa///",    final: "/alexa/" },
+      { original: "/alexa/",     final: "/alexa/" },
+      { original: "//alexa//",   final: "/alexa/" },
+      { original: "///alexa///", final: "/alexa/" },
+      { original: "///api///alexa///", final: "/api/alexa/" }
+    ];
+
+    tests.forEach(function(test) {
+      it('correctly normalizes ' + test.original + ' into ' + test.final, function() {
+        expect(utils.normalizeApiPath(test.original)).to.equal(test.final);
+      });
+    });
+  });
+});

utils.js

@@ -0,0 +1,30 @@
+var fs = require('fs');
+var path = require('path');
+
+var isValidDirectory = function(dir) {
+  return fs.existsSync(dir) && fs.statSync(dir).isDirectory();
+};
+
+var isValidFile = function(file) {
+  return fs.existsSync(file) && fs.statSync(file).isFile();
+};
+
+var readFile = function(file) {
+  return fs.readFileSync(file, 'utf8');
+};
+
+var readJsonFile = function(file) {
+  return JSON.parse(readFile(file));
+};
+
+var normalizeApiPath = function(apiPath) {
+  return path.posix.normalize(path.posix.join('/', apiPath));
+};
+
+module.exports = {
+  isValidDirectory  : isValidDirectory,
+  isValidFile       : isValidFile,
+  readFile          : readFile,
+  readJsonFile      : readJsonFile,
+  normalizeApiPath  : normalizeApiPath
+};