Merge pull request #183 from probot/express

Use Express for the web server and allow plugins add routes

bin/probot-simulate.js

@@ -33,5 +33,5 @@ const plugins = require('../lib/plugin')(probot);
 
 plugins.load(program.args.slice(2));
 
-probot.robot.log('Simulating event', eventName);
+probot.logger.debug('Simulating event', eventName);
 probot.receive({event: eventName, payload});

docs/plugins.md

@@ -90,6 +90,31 @@ $ probot run -a APP_ID -P private-key.pem ./index.js
 Listening on http://localhost:3000
 ```
 
+## HTTP Routes
+
+Calling `robot.route('/my-plugin')` will return an [express](http://expressjs.com/) router that you can use to expose HTTP endpoints from your plugin.
+
+```js
+module.exports = robot => {
+  // Get an express router to expose new HTTP endpoints
+  const app = robot.route('/my-plugin');
+
+  // Use any middleware
+  app.use(require('express').static(__dirname + '/public'));
+
+  // Add a new route
+  app.get('/hello-world', (req, res) => {
+    res.end('Hello World');
+  });
+};
+```
+
+Visit https://localhost:3000/my-plugin/hello-world to access the endpoint.
+
+It is strongly encouraged to use the name of your package as the prefix so none of your routes or middleware conflict with other plugins. For example, if [`probot/owners`](https://github.com/probot/owners) exposed an endpoint, the plugin would call `robot.route('/owners')` to prefix all endpoints with `/owners`.
+
+See the [express documentation](http://expressjs.com/en/guide/routing.html) for more information.
+
 ## Simulating webhooks
 
 As you are developing your plugin, you will likely want to test it by repeatedly trigging the same webhook. You can simulate a webhook being delivered by saving the payload to a file, and then calling `probot simulate` from the command line.

index.js

@@ -31,12 +31,11 @@ module.exports = (options = {}) => {
     debug: process.env.LOG_LEVEL === 'trace'
   });
   const server = createServer(webhook);
-  const robot = createRobot({app, webhook, cache, logger, catchErrors: true});
 
-  // Forward webhooks to robot
+  // Log all received webhooks
   webhook.on('*', event => {
     logger.trace(event, 'webhook received');
-    robot.receive(event);
+    receive(event);
   });
 
   // Log all webhook errors
@@ -55,10 +54,25 @@ module.exports = (options = {}) => {
     logger.addStream(sentryStream(Raven));
   }
 
+  const robots = [];
+
+  function receive(event) {
+    return Promise.all(robots.map(robot => robot.receive(event)));
+  }
+
   return {
     server,
-    robot,
     webhook,
+    receive,
+    logger,
+
+    // Return the first robot
+    get robot() {
+      const caller = (new Error()).stack.split('\n')[2];
+      console.warn('DEPRECATED: the `robot` property is deprecated and will be removed in 0.10.0');
+      console.warn(caller);
+      return robots[0] || createRobot({app, cache, logger, catchErrors: true});
+    },
 
     start() {
       server.listen(options.port);
@@ -66,11 +80,16 @@ module.exports = (options = {}) => {
     },
 
     load(plugin) {
-      plugin(robot);
-    },
+      const robot = createRobot({app, cache, logger, catchErrors: true});
 
-    receive(event) {
-      return robot.receive(event);
+      // Connect the router from the robot to the server
+      server.use(robot.router);
+
+      // Initialize the plugin
+      plugin(robot);
+      robots.push(robot);
+
+      return robot;
     }
   };
 };

lib/plugin.js

@@ -49,7 +49,7 @@ function pluginLoaderFactory(probot, opts = {}) {
     const plugins = autoloader('probot-*');
     Object.keys(plugins).forEach(pluginName => {
       loadPlugin(pluginName, plugins[pluginName]);
-      probot.robot.log.info(`Automatically loaded plugin: ${pluginName}`);
+      probot.logger.info(`Automatically loaded plugin: ${pluginName}`);
     });
   }
 
@@ -61,7 +61,7 @@ function pluginLoaderFactory(probot, opts = {}) {
     pluginNames.forEach(pluginName => {
       const pluginPath = resolvePlugin(pluginName);
       loadPlugin(pluginName, pluginPath);
-      probot.robot.log.debug(`Loaded plugin: ${pluginName}`);
+      probot.logger.debug(`Loaded plugin: ${pluginName}`);
     });
   }
 

lib/robot.js

@@ -1,6 +1,7 @@
 const {EventEmitter} = require('promise-events');
 const GitHubApi = require('github');
 const Bottleneck = require('bottleneck');
+const express = require('express');
 const Context = require('./context');
 
 /**
@@ -9,10 +10,11 @@ const Context = require('./context');
  * @property {logger} log - A logger
  */
 class Robot {
-  constructor({app, cache, logger, catchErrors} = {}) {
+  constructor({app, cache, logger, router, catchErrors} = {}) {
     this.events = new EventEmitter();
     this.app = app;
     this.cache = cache;
+    this.router = router || new express.Router();
     this.log = wrapLogger(logger);
     this.catchErrors = catchErrors;
   }
@@ -23,6 +25,37 @@ class Robot {
     });
   }
 
+  /**
+   * Get an {@link http://expressjs.com|express} router that can be used to
+   * expose HTTP endpoints
+   *
+   * @example
+   * module.exports = robot => {
+   *   // Get an express router to expose new HTTP endpoints
+   *   const app = robot.route('/my-plugin');
+   *
+   *   // Use any middleware
+   *   app.use(require('express').static(__dirname + '/public'));
+   *
+   *   // Add a new route
+   *   app.get('/hello-world', (req, res) => {
+   *     res.end('Hello World');
+   *   });
+   * };
+   *
+   * @param {string} path - the prefix for the routes
+   * @returns {@link http://expressjs.com/en/4x/api.html#router|express.Router}
+   */
+  route(path) {
+    if (path) {
+      const router = new express.Router();
+      this.router.use(path, router);
+      return router;
+    } else {
+      return this.router;
+    }
+  }
+
   /**
    * Listen for [GitHub webhooks](https://developer.github.com/webhooks/),
    * which are fired for almost every significant action that users take on

lib/server.js

@@ -1,18 +1,10 @@
-const http = require('http');
+const express = require('express');
 
 module.exports = function (webhook) {
-  return http.createServer((req, res) => {
-    webhook(req, res, err => {
-      if (err) {
-        console.error(err);
-        res.statusCode = 500;
-        res.end('Something has gone terribly wrong.');
-      } else if (req.url.split('?').shift() === '/ping') {
-        res.end('PONG');
-      } else {
-        res.statusCode = 404;
-        res.end('no such location');
-      }
-    });
-  });
+  const app = express();
+
+  app.use(webhook);
+  app.get('/ping', (req, res) => res.end('PONG'));
+
+  return app;
 };

package.json

@@ -23,6 +23,7 @@
     "cache-manager": "^2.4.0",
     "commander": "^2.10.0",
     "dotenv": "~4.0.0",
+    "express": "^4.15.3",
     "github": "^9.2.0",
     "github-app": "^3.0.0",
     "github-webhook-handler": "^0.6.0",
@@ -41,6 +42,7 @@
     "jsdoc-strip-async-await": "^0.1.0",
     "minami": "^1.1.1",
     "mocha": "^3.0.2",
+    "supertest": "^3.0.0",
     "xo": "^0.19.0"
   },
   "xo": {

test/fixtures/example.js

@@ -0,0 +1,3 @@
+module.exports = robot => {
+  console.log('laoded plugin');
+}

test/index.js

@@ -7,8 +7,6 @@ describe('Probot', () => {
 
   beforeEach(() => {
     probot = createProbot();
-    // Mock out GitHub App authentication
-    probot.robot.auth = () => Promise.resolve({});
 
     event = {
       event: 'push',
@@ -18,9 +16,84 @@ describe('Probot', () => {
 
   describe('webhook delivery', () => {
     it('forwards webhooks to the robot', async () => {
-      probot.robot.receive = expect.createSpy();
+      const robot = probot.load(() => {});
+      robot.receive = expect.createSpy();
       probot.webhook.emit('*', event);
-      expect(probot.robot.receive).toHaveBeenCalledWith(event);
+      expect(robot.receive).toHaveBeenCalledWith(event);
+    });
+  });
+
+  describe('server', () => {
+    const request = require('supertest');
+
+    it('prefixes paths with route name', () => {
+      probot.load(robot => {
+        const app = robot.route('/my-plugin');
+        app.get('/foo', (req, res) => res.end('foo'));
+      });
+
+      return request(probot.server).get('/my-plugin/foo').expect(200, 'foo');
+    });
+
+    it('allows routes with no path', () => {
+      probot.load(robot => {
+        const app = robot.route();
+        app.get('/foo', (req, res) => res.end('foo'));
+      });
+
+      return request(probot.server).get('/foo').expect(200, 'foo');
+    });
+
+    it('isolates plugins from affecting eachother', async () => {
+      ['foo', 'bar'].forEach(name => {
+        probot.load(robot => {
+          const app = robot.route('/' + name);
+
+          app.use(function (req, res, next) {
+            res.append('X-Test', name);
+            next();
+          });
+
+          app.get('/hello', (req, res) => res.end(name));
+        });
+      });
+
+      await request(probot.server).get('/foo/hello')
+        .expect(200, 'foo')
+        .expect('X-Test', 'foo');
+
+      await request(probot.server).get('/bar/hello')
+        .expect(200, 'bar')
+        .expect('X-Test', 'bar');
+    });
+  });
+
+  describe('receive', () => {
+    it('forwards events to each plugin', async () => {
+      const spy = expect.createSpy();
+      const robot = probot.load(robot => robot.on('push', spy));
+      robot.auth = expect.createSpy().andReturn(Promise.resolve({}));
+
+      await probot.receive(event);
+
+      expect(spy).toHaveBeenCalled();
+    });
+  });
+
+  describe('robot', () => {
+    it('will be removed in 0.10', () => {
+      const semver = require('semver');
+      const pkg = require('../package');
+      expect(semver.satisfies(pkg.version, '>=0.9')).toBe(false, 'remove in 0.10.0');
+    });
+
+    it('returns the first defined (for now)', () => {
+      const robot = probot.load(() => { });
+      expect(probot.robot).toBe(robot);
+    });
+
+    it('returns a robot if no plugins are loaded', () => {
+      expect(probot.robot).toExist();
     });
   });
 });

test/plugins.js

@@ -20,9 +20,7 @@ describe('plugin loader', function () {
   beforeEach(function () {
     probot = {
       load: expect.createSpy(),
-      robot: {
-        log: nullLogger
-      }
+      logger: nullLogger
     };
 
     autoplugins = {

test/server.js

@@ -0,0 +1,32 @@
+const expect = require('expect');
+const request = require('supertest');
+const createServer = require('../lib/server');
+
+describe('server', function () {
+  let server;
+  let webhook;
+
+  beforeEach(() => {
+    webhook = expect.createSpy().andCall((req, res, next) => next());
+    server = createServer(webhook);
+  });
+
+  describe('GET /ping', () => {
+    it('returns a 200 repsonse', () => {
+      return request(server).get('/ping').expect(200, 'PONG');
+    });
+  });
+
+  describe('webhook handler', () => {
+    it('should 500 on a webhook error', () => {
+      webhook.andCall((req, res, callback) => callback('webhook error'));
+      return request(server).post('/').expect(500);
+    });
+  });
+
+  describe('with an unknown url', () => {
+    it('responds with 404', () => {
+      return request(server).get('/lolnotfound').expect(404);
+    });
+  });
+});