[RFC] New API for registering services

.eslintrc.yml

@@ -1,3 +1,6 @@
+parserOptions:
+  ecmaVersion: 8
+
 env:
   node: true
   # We use Promise, Map, and occasional ES6 syntax.

.travis.yml

@@ -2,7 +2,7 @@ language: node_js
 
 node_js:
   - 8
-  - 6
+  # - 9
 
 cache:
   directories:
@@ -17,19 +17,19 @@ before_script:
 script:
   - npm run lint
   - npm run test:js:server
-  - if node_modules/.bin/check-node-version --node '< 8.0' > /dev/null; then echo "Skipping frontend tests."; else npm run test:js:frontend; fi
-  - if node_modules/.bin/check-node-version --node '< 8.0' > /dev/null; then echo "Skipping build."; else npm run build; fi
+  - npm run test:js:frontend
+  - npm run build
 
 jobs:
   include:
     - node_js: 8
       script:
       - if [ "$TRAVIS_EVENT_TYPE" == cron ]; then npm run test:services; fi
       - if [ "$TRAVIS_EVENT_TYPE" == pull_request ]; then npm run test:services:pr; fi
-    - node_js: 6
-      script:
-      - if [ "$TRAVIS_EVENT_TYPE" == cron ]; then npm run test:services; fi
-      - if [ "$TRAVIS_EVENT_TYPE" == pull_request ]; then npm run test:services:pr; fi
+    # - node_js: 9
+    #   script:
+    #   - if [ "$TRAVIS_EVENT_TYPE" == cron ]; then npm run test:services; fi
+    #   - if [ "$TRAVIS_EVENT_TYPE" == pull_request ]; then npm run test:services:pr; fi
 
 branches:
   except:

lib/request-handler.js

@@ -166,6 +166,18 @@ function handleRequest (handlerOptions) {
       });
     }
 
+   // Wrapper around `cachingRequest` that returns a promise rather than
+   // needing to pass a callback.
+   cachingRequest.asPromise = (uri, options) => new Promise((resolve, reject) => {
+     cachingRequest(uri, options, (err, res, buffer) => {
+       if (err) {
+         reject(err);
+       } else {
+         resolve({res, buffer});
+       }
+     });
+   });
+
     vendorDomain.run(() => {
       handlerOptions.handler(filteredQueryParams, match, function sendBadge(format, badgeData) {
         if (serverUnresponsive) { return; }

package.json

@@ -50,7 +50,7 @@
     "coverage:report:open": "npm run coverage:report && npm run coverage:report:reopen",
     "lint": "eslint '**/*.js'",
     "test:js:frontend": "mocha --require babel-polyfill --require babel-register 'frontend/**/*.spec.js'",
-    "test:js:server": "mocha '*.spec.js' 'lib/**/*.spec.js'",
+    "test:js:server": "mocha '*.spec.js' 'lib/**/*.spec.js' 'services/**/*.spec.js'",
     "test:services": "mocha --delay service-tests/runner/cli.js",
     "test:services:pr:prepare": "node service-tests/runner/pull-request-services-cli.js > pull-request-services.log",
     "test:services:pr:run": "mocha --delay service-tests/runner/cli.js --stdin < pull-request-services.log",

server.js

@@ -4,6 +4,7 @@ const countBy = require('lodash.countby');
 const jp = require('jsonpath');
 const path = require('path');
 const prettyBytes = require('pretty-bytes');
+const glob = require('glob');
 const queryString = require('query-string');
 const semver = require('semver');
 const xml2js = require('xml2js');
@@ -172,6 +173,12 @@ camp.notfound(/.*/, function(query, match, end, request) {
 
 // Vendors.
 
+// New-style services
+glob.sync(`${__dirname}/services/*.js`)
+  .filter(path => !path.endsWith('base.js') && !path.endsWith('.spec.js'))
+  .map(path => require(path))
+  .forEach(serviceClass => serviceClass.register(camp, cache));
+
 // JIRA issue integration
 camp.route(/^\/jira\/issue\/(http(?:s)?)\/(.+)\/([^/]+)\.(svg|png|gif|jpg|json)$/,
 cache(function (data, match, sendBadge, request) {
@@ -680,48 +687,6 @@ cache(function (data, match, sendBadge, request) {
   });
 }));
 
-// AppVeyor CI integration.
-camp.route(/^\/appveyor\/ci\/([^/]+\/[^/]+)(?:\/(.+))?\.(svg|png|gif|jpg|json)$/,
-cache(function(data, match, sendBadge, request) {
-  var repo = match[1];  // eg, `gruntjs/grunt`.
-  var branch = match[2];
-  var format = match[3];
-  var apiUrl = 'https://ci.appveyor.com/api/projects/' + repo;
-  if (branch != null) {
-    apiUrl += '/branch/' + branch;
-  }
-  var badgeData = getBadgeData('build', data);
-  request(apiUrl, { headers: { 'Accept': 'application/json' } }, function(err, res, buffer) {
-    if (err != null) {
-      badgeData.text[1] = 'inaccessible';
-      sendBadge(format, badgeData);
-      return;
-    }
-    try {
-      if (res.statusCode === 404) {
-        badgeData.text[1] = 'project not found or access denied';
-        sendBadge(format, badgeData);
-        return;
-      }
-      var data = JSON.parse(buffer);
-      var status = data.build.status;
-      if (status === 'success') {
-        badgeData.text[1] = 'passing';
-        badgeData.colorscheme = 'brightgreen';
-      } else if (status !== 'running' && status !== 'queued') {
-        badgeData.text[1] = 'failing';
-        badgeData.colorscheme = 'red';
-      } else {
-        badgeData.text[1] = status;
-      }
-      sendBadge(format, badgeData);
-    } catch(e) {
-      badgeData.text[1] = 'invalid';
-      sendBadge(format, badgeData);
-    }
-  });
-}));
-
 // AppVeyor test status integration.
 camp.route(/^\/appveyor\/tests\/([^/]+\/[^/]+)(?:\/(.+))?\.(svg|png|gif|jpg|json)$/,
 cache(function(data, match, sendBadge, request) {

service-tests/runner/pull-request-services-cli.js

@@ -42,7 +42,7 @@ function servicesForTitle (title) {
   }
 
   const services = matches[1].toLowerCase().split(' ');
-  const blacklist = ['wip'];
+  const blacklist = ['wip', 'rfc'];
   return difference(services, blacklist);
 }
 

services/appveyor.js

@@ -0,0 +1,56 @@
+'use strict';
+
+const BaseService = require('./base');
+
+/**
+ * AppVeyor CI integration.
+ */
+module.exports = class AppVeyor extends BaseService {
+  async handle({repo, branch}) {
+    let apiUrl = 'https://ci.appveyor.com/api/projects/' + repo;
+    if (branch != null) {
+      apiUrl += '/branch/' + branch;
+    }
+    const {buffer, res} = await this._sendAndCacheRequest(apiUrl, {
+      headers: { 'Accept': 'application/json' }
+    });
+
+    if (res.statusCode === 404) {
+      return {text: 'project not found or access denied'};
+    }
+
+    const data = JSON.parse(buffer);
+    const status = data.build.status;
+    if (status === 'success') {
+      return {text: 'passing', colorscheme: 'brightgreen'};
+    } else if (status !== 'running' && status !== 'queued') {
+      return {text: 'failing', colorscheme: 'red'};
+    } else {
+      return {text: status};
+    }
+  }
+
+  // Metadata
+  static get category() {
+    return 'build';
+  }
+
+  static get uri() {
+    return {
+      format: '/appveyor/ci/([^/]+/[^/]+)(?:/(.+))?',
+      capture: ['repo', 'branch']
+    };
+  }
+
+  static getExamples() {
+    return [
+      {
+        uri: '/appveyor/ci/gruntjs/grunt',
+      },
+      {
+        name: 'Branch',
+        uri: '/appveyor/ci/gruntjs/grunt/master',
+      },
+    ];
+  }
+};

services/base.js

@@ -0,0 +1,112 @@
+'use strict';
+
+const {
+  makeBadgeData: getBadgeData,
+} = require('../lib/badge-data');
+
+module.exports = class BaseService {
+  constructor({sendAndCacheRequest}) {
+    this._sendAndCacheRequest = sendAndCacheRequest;
+  }
+
+  /**
+   * Asynchronous function to handle requests for this service. Takes the URI
+   * parameters (as defined in the `uri` property), performs a request using
+   * `this._sendAndCacheRequest`, and returns the badge data.
+   */
+  async handle(namedParams) {
+    throw new Error(
+      `Handler not implemented for ${this.constructor.name}`
+    );
+  }
+
+  // Metadata
+
+  /**
+   * Name of the category to sort this badge into (eg. "build"). Used to sort
+   * the badges on the main shields.io website.
+   */
+  static get category() {
+    return 'unknown';
+  }
+  /**
+   * Returns an object with two fields:
+   *  - format: Regular expression to use for URIs for this service's badges
+   *  - capture: Array of names for the capture groups in the regular
+   *             expression. The handler will be passed an object containing
+   *             the matches.
+   */
+  static get uri() {
+    throw new Error(`URI not defined for ${this.name}`);
+  }
+
+  /**
+   * Default data for the badge. Can include things such as default logo, color,
+   * etc. These defaults will be used if the value is not explicitly overridden
+   * by either the handler or by the user via URL parameters.
+   */
+  static get defaultBadgeData() {
+    return {};
+  }
+
+  /**
+   * Example URIs for this service. These should use the format
+   * specified in `uri`, and can be used to demonstrate how to use badges for
+   * this service.
+   */
+  static getExamples() {
+    return [];
+  }
+
+  static register(camp, handleRequest) {
+    const serviceClass = this; // In a static context, "this" is the class.
+
+    // Regular expressions treat "/" specially, so we need to escape them
+    const escapedPath = serviceClass.uri.format.replace(/\//g, '\\/');
+    const fullRegex = '^' + escapedPath + '.(svg|png|gif|jpg|json)$';
+
+    camp.route(new RegExp(fullRegex),
+    handleRequest(async (data, match, sendBadge, request) => {
+      // Assumes the final capture group is the extension
+      const format = match.pop();
+      const badgeData = getBadgeData(
+        serviceClass.category,
+        Object.assign({}, serviceClass.defaultBadgeData, data)
+      );
+
+      try {
+        const namedParams = {};
+        if (serviceClass.uri.capture.length !== match.length - 1) {
+          throw new Error(
+            `Incorrect number of capture groups (expected `+
+            `${serviceClass.uri.capture.length}, got ${match.length - 1})`
+          );
+        }
+
+        serviceClass.uri.capture.forEach((name, index) => {
+          // The first capture group is the entire match, so every index is + 1 here
+          namedParams[name] = match[index + 1];
+        });
+
+        const serviceInstance = new serviceClass({
+          sendAndCacheRequest: request.asPromise,
+        });
+        const serviceData = await serviceInstance.handle(namedParams);
+        const text = badgeData.text;
+        if (serviceData.text) {
+          text[1] = serviceData.text;
+        }
+        Object.assign(badgeData, serviceData);
+        badgeData.text = text;
+        sendBadge(format, badgeData);
+
+      } catch (error) {
+        console.log(error);
+        const text = badgeData.text;
+        text[1] = 'error';
+        badgeData.text = text;
+        sendBadge(format, badgeData);
+      }
+    }));
+  }
+};

services/base.spec.js

@@ -0,0 +1,73 @@
+'use strict';
+
+const assert = require('assert');
+const sinon = require('sinon');
+
+const BaseService = require('./base');
+
+class DummyService extends BaseService {
+  async handle({someArg}) {
+    return {
+      text: 'Hello ' + someArg,
+    };
+  }
+
+  static get category() { return 'cat'; }
+  static get uri() {
+    return {
+      format: '/foo/([^/]+)',
+      capture: ['someArg']
+    };
+  }
+}
+
+const expectedRouteRegex = /^\/foo\/([^/]+).(svg|png|gif|jpg|json)$/;
+
+describe('BaseService', () => {
+  let mockCamp;
+  let mockHandleRequest;
+
+  beforeEach(() => {
+    mockCamp = {
+      route: sinon.spy(),
+    };
+    mockHandleRequest = sinon.spy();
+    DummyService.register(mockCamp, mockHandleRequest);
+  });
+
+  it('registers the service', () => {
+    assert(mockCamp.route.calledOnce);
+    assert.equal(mockCamp.route.getCall(0).args[0].toString(), expectedRouteRegex);
+  });
+
+  it('handles the request', async () => {
+    assert(mockHandleRequest.calledOnce);
+    const requestHandler = mockHandleRequest.getCall(0).args[0];
+
+    const mockSendBadge = sinon.spy();
+    const mockRequest = {
+      asPromise: sinon.spy(),
+    };
+    await requestHandler(
+      /*data*/ {},
+      /*match*/ '/foo/bar.svg'.match(expectedRouteRegex),
+      mockSendBadge,
+      mockRequest
+    );
+
+    assert(mockSendBadge.calledOnce);
+    assert(mockSendBadge.calledWith(
+      /*format*/ 'svg',
+      {
+        text: ['cat', 'Hello bar'],
+        colorscheme: 'lightgrey',
+        template: 'default',
+        logo: undefined,
+        logoWidth: NaN,
+        links: [],
+        colorA: undefined,
+        colorB: undefined,
+      }
+    ));
+  });
+});