Mobrovac has uploaded a new change for review. ( https://gerrit.wikimedia.org/r/398031 )
Change subject: Update to service-template-node v0.5.4 ...................................................................... Update to service-template-node v0.5.4 Bug: T151392 Change-Id: I21bb4113a19810e04e099fd7edcade44450ba6af --- A .eslintrc.yml M .travis.yml M app.js M lib/api-util.js A lib/swagger-ui.js M lib/util.js M lib/vega.js M lib/vega1.js M lib/vega2.js M package.json M routes/graphoid-v1.js M routes/graphoid-v2.js M routes/info.js M routes/root.js M server.js M test/features/app/app.js M test/features/app/spec.js M test/features/info/info.js M test/index.js M test/utils/assert.js M test/utils/logStream.js M test/utils/server.js 22 files changed, 767 insertions(+), 650 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/services/graphoid refs/changes/31/398031/1 diff --git a/.eslintrc.yml b/.eslintrc.yml new file mode 100644 index 0000000..9e2c225 --- /dev/null +++ b/.eslintrc.yml @@ -0,0 +1 @@ +extends: 'eslint-config-node-services' \ No newline at end of file diff --git a/.travis.yml b/.travis.yml index 34ca0bf..b6a9e5d 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,8 +3,7 @@ sudo: false node_js: - - "0.10" - - "0.12" - "4" - "6" + - "8" - "node" diff --git a/app.js b/app.js index fb8bc35..9a5a618 100644 --- a/app.js +++ b/app.js @@ -1,18 +1,17 @@ 'use strict'; -require('core-js/shim'); - -var http = require('http'); -var BBPromise = require('bluebird'); -var express = require('express'); -var compression = require('compression'); -var bodyParser = require('body-parser'); -var fs = BBPromise.promisifyAll(require('fs')); -var sUtil = require('./lib/util'); -var apiUtil = require('./lib/api-util'); -var packageInfo = require('./package.json'); -var yaml = require('js-yaml'); +const http = require('http'); +const BBPromise = require('bluebird'); +const express = require('express'); +const compression = require('compression'); +const bodyParser = require('body-parser'); +const fs = BBPromise.promisifyAll(require('fs')); +const sUtil = require('./lib/util'); +const apiUtil = require('./lib/api-util'); +const packageInfo = require('./package.json'); +const yaml = require('js-yaml'); +const addShutdown = require('http-shutdown'); /** @@ -23,7 +22,7 @@ function initApp(options) { // the main application object - var app = express(); + const app = express(); // get the options and make them available in the app app.logger = options.logger; // the logging device @@ -32,22 +31,22 @@ app.info = packageInfo; // this app's package info // ensure some sane defaults - if(!app.conf.port) { app.conf.port = 8888; } - if(!app.conf.interface) { app.conf.interface = '0.0.0.0'; } - if(app.conf.compression_level === undefined) { app.conf.compression_level = 3; } - if(app.conf.cors === undefined) { app.conf.cors = '*'; } - if(app.conf.csp === undefined) { - app.conf.csp = - "default-src 'self'; object-src 'none'; media-src *; img-src *; style-src *; frame-ancestors 'self'"; + if (!app.conf.port) { app.conf.port = 8888; } + if (!app.conf.interface) { app.conf.interface = '0.0.0.0'; } + if (app.conf.compression_level === undefined) { app.conf.compression_level = 3; } + if (app.conf.cors === undefined) { app.conf.cors = '*'; } + if (app.conf.csp === undefined) { + // eslint-disable-next-line max-len + app.conf.csp = "default-src 'self'; object-src 'none'; media-src *; img-src *; style-src *; frame-ancestors 'self'"; } // set outgoing proxy - if(app.conf.proxy) { + if (app.conf.proxy) { process.env.HTTP_PROXY = app.conf.proxy; // if there is a list of domains which should // not be proxied, set it - if(app.conf.no_proxy_list) { - if(Array.isArray(app.conf.no_proxy_list)) { + if (app.conf.no_proxy_list) { + if (Array.isArray(app.conf.no_proxy_list)) { process.env.NO_PROXY = app.conf.no_proxy_list.join(','); } else { process.env.NO_PROXY = app.conf.no_proxy_list; @@ -56,35 +55,35 @@ } // set up header whitelisting for logging - if(!app.conf.log_header_whitelist) { + if (!app.conf.log_header_whitelist) { app.conf.log_header_whitelist = [ - 'cache-control', 'content-type', 'content-length', 'if-match', - 'user-agent', 'x-request-id' + 'cache-control', 'content-type', 'content-length', 'if-match', + 'user-agent', 'x-request-id' ]; } - app.conf.log_header_whitelist = new RegExp('^(?:' + app.conf.log_header_whitelist.map(function(item) { + app.conf.log_header_whitelist = new RegExp(`^(?:${app.conf.log_header_whitelist.map((item) => { return item.trim(); - }).join('|') + ')$', 'i'); + }).join('|')})$`, 'i'); // set up the request templates for the APIs apiUtil.setupApiTemplates(app); // set up the spec - if(!app.conf.spec) { - app.conf.spec = __dirname + '/spec.yaml'; + if (!app.conf.spec) { + app.conf.spec = `${__dirname}/spec.yaml`; } - if(app.conf.spec.constructor !== Object) { + if (app.conf.spec.constructor !== Object) { try { app.conf.spec = yaml.safeLoad(fs.readFileSync(app.conf.spec)); - } catch(e) { - app.logger.log('warn/spec', 'Could not load the spec: ' + e); + } catch (e) { + app.logger.log('warn/spec', `Could not load the spec: ${e}`); app.conf.spec = {}; } } - if(!app.conf.spec.swagger) { + if (!app.conf.spec.swagger) { app.conf.spec.swagger = '2.0'; } - if(!app.conf.spec.info) { + if (!app.conf.spec.info) { app.conf.spec.info = { version: app.info.version, title: app.info.name, @@ -92,18 +91,18 @@ }; } app.conf.spec.info.version = app.info.version; - if(!app.conf.spec.paths) { + if (!app.conf.spec.paths) { app.conf.spec.paths = {}; } // set the CORS and CSP headers - app.all('*', function(req, res, next) { - if(app.conf.cors !== false) { + app.all('*', (req, res, next) => { + if (app.conf.cors !== false) { res.header('access-control-allow-origin', app.conf.cors); res.header('access-control-allow-headers', 'accept, x-requested-with, content-type'); res.header('access-control-expose-headers', 'etag'); } - if(app.conf.csp !== false) { + if (app.conf.csp !== false) { res.header('x-xss-protection', '1; mode=block'); res.header('x-content-type-options', 'nosniff'); res.header('x-frame-options', 'SAMEORIGIN'); @@ -123,11 +122,11 @@ // disable the ETag header - users should provide them! app.set('etag', false); // enable compression - app.use(compression({level: app.conf.compression_level})); + app.use(compression({ level: app.conf.compression_level })); // use the JSON body parser app.use(bodyParser.json()); // use the application/x-www-form-urlencoded parser - app.use(bodyParser.urlencoded({extended: true})); + app.use(bodyParser.urlencoded({ extended: true })); return BBPromise.resolve(app); @@ -137,45 +136,46 @@ /** * Loads all routes declared in routes/ into the app * @param {Application} app the application object to load routes into - * @returns {bluebird} a promise resolving to the app object + * @return {bluebird} a promise resolving to the app object */ -function loadRoutes (app) { +function loadRoutes(app) { // get the list of files in routes/ - return fs.readdirAsync(__dirname + '/routes').map(function(fname) { - return BBPromise.try(function() { + return fs.readdirAsync(`${__dirname}/routes`).map((fname) => { + return BBPromise.try(() => { // ... and then load each route // but only if it's a js file - if(!/\.js$/.test(fname)) { + if (!/\.js$/.test(fname)) { return undefined; } // import the route file - var route = require(__dirname + '/routes/' + fname); + const route = require(`${__dirname}/routes/${fname}`); return route(app); - }).then(function(route) { - if(route === undefined) { + }).then((route) => { + if (route === undefined) { return undefined; } // check that the route exports the object we need - if(route.constructor !== Object || !route.path || !route.router || !(route.api_version || route.skip_domain)) { - throw new TypeError('routes/' + fname + ' does not export the correct object!'); + if (route.constructor !== Object || !route.path || !route.router + || !(route.api_version || route.skip_domain)) { + throw new TypeError(`routes/${fname} does not export the correct object!`); } // normalise the path to be used as the mount point - if(route.path[0] !== '/') { - route.path = '/' + route.path; + if (route.path[0] !== '/') { + route.path = `/${route.path}`; } - if(route.path[route.path.length - 1] !== '/') { - route.path = route.path + '/'; + if (route.path[route.path.length - 1] !== '/') { + route.path = `${route.path}/`; } - if(!route.skip_domain) { - route.path = '/:domain/v' + route.api_version + route.path; + if (!route.skip_domain) { + route.path = `/:domain/v${route.api_version}${route.path}`; } // wrap the route handlers with Promise.try() blocks sUtil.wrapRouteHandlers(route, app); // all good, use that route app.use(route.path, route.router); }); - }).then(function () { + }).then(() => { // catch errors sUtil.setErrorHandler(app); // route loading is now complete, return the app object @@ -188,23 +188,32 @@ /** * Creates and start the service's web server * @param {Application} app the app object to use in the service - * @returns {bluebird} a promise creating the web server + * @return {bluebird} a promise creating the web server */ function createServer(app) { // return a promise which creates an HTTP server, // attaches the app to it, and starts accepting // incoming client requests - var server; - return new BBPromise(function (resolve) { + let server; + return new BBPromise((resolve) => { server = http.createServer(app).listen( app.conf.port, app.conf.interface, resolve ); - }).then(function () { + server = addShutdown(server); + }).then(() => { app.logger.log('info', - 'Worker ' + process.pid + ' listening on ' + (app.conf.interface || '*') + ':' + app.conf.port); + `Worker ${process.pid} listening on ${app.conf.interface || '*'}:${app.conf.port}`); + + // Don't delay incomplete packets for 40ms (Linux default) on + // pipelined HTTP sockets. We write in large chunks or buffers, so + // lack of coalescing should not be an issue here. + server.on("connection", (socket) => { + socket.setNoDelay(true); + }); + return server; }); @@ -221,9 +230,9 @@ return initApp(options) .then(loadRoutes) - .then(function(app) { + .then((app) => { // serve static files from static/ - app.use('/static', express.static(__dirname + '/static')); + app.use('/static', express.static(`${__dirname}/static`)); return app; }).then(createServer); diff --git a/lib/api-util.js b/lib/api-util.js index 998f2bd..8e12e8d 100644 --- a/lib/api-util.js +++ b/lib/api-util.js @@ -1,36 +1,34 @@ 'use strict'; -var preq = require('preq'); -var sUtil = require('../lib/util'); -var Template = require('swagger-router').Template; - -var HTTPError = sUtil.HTTPError; +const preq = require('preq'); +const sUtil = require('./util'); +const Template = require('swagger-router').Template; +const HTTPError = sUtil.HTTPError; /** * Calls the MW API with the supplied query as its body - * - * @param {Object} app the application object + * @param {!Object} app the application object * @param {string} domain the domain to issue the request to - * @param {Object} query an object with all the query parameters for the MW API - * @return {Promise} a promise resolving as the response object from the MW API + * @param {?Object} query an object with all the query parameters for the MW API + * @return {!Promise} a promise resolving as the response object from the MW API */ function mwApiGet(app, domain, query) { query = query || {}; query.continue = query.continue || ''; - var request = app.mwapi_tpl.expand({ + const request = app.mwapi_tpl.expand({ request: { - params: { domain: domain }, + params: { domain }, headers: { 'user-agent': app.conf.user_agent }, - query: query + query } }); - return preq(request).then(function(response) { - if(response.status < 200 || response.status > 399) { + return preq(request).then((response) => { + if (response.status < 200 || response.status > 399) { // there was an error when calling the upstream service, propagate that throw new HTTPError({ status: response.status, @@ -47,16 +45,15 @@ /** * Calls the REST API with the supplied domain, path and request parameters - * - * @param {Object} app the application object + * @param {!Object} app the application object * @param {string} domain the domain to issue the request for - * @param {string} path the REST API path to contact without the leading slash - * @param {Object} [restReq={}] the object containing the REST request details - * @param {string} [restReq.method=get] the request method - * @param {Object} [restReq.query={}] the query string to send, if any - * @param {Object} [restReq.headers={}] the request headers to send - * @param {Object} [restReq.body=null] the body of the request, if any - * @return {Promise} a promise resolving as the response object from the REST API + * @param {!string} path the REST API path to contact without the leading slash + * @param {?Object} [restReq={}] the object containing the REST request details + * @param {?string} [restReq.method=get] the request method + * @param {?Object} [restReq.query={}] the query string to send, if any + * @param {?Object} [restReq.headers={}] the request headers to send + * @param {?Object} [restReq.body=null] the body of the request, if any + * @return {!Promise} a promise resolving as the response object from the REST API * */ function restApiGet(app, domain, path, restReq) { @@ -64,10 +61,10 @@ restReq = restReq || {}; path = path[0] === '/' ? path.slice(1) : path; - var request = app.restbase_tpl.expand({ + const request = app.restbase_tpl.expand({ request: { method: restReq.method, - params: { domain: domain, path: path }, + params: { domain, path }, query: restReq.query, headers: Object.assign({ 'user-agent': app.conf.user_agent }, restReq.headers), body: restReq.body @@ -81,13 +78,12 @@ /** * Sets up the request templates for MW and RESTBase API requests - * - * @param {Application} app the application object + * @param {!Application} app the application object */ function setupApiTemplates(app) { // set up the MW API request template - if(!app.conf.mwapi_req) { + if (!app.conf.mwapi_req) { app.conf.mwapi_req = { uri: 'http://{{domain}}/w/api.php', headers: { @@ -99,7 +95,7 @@ app.mwapi_tpl = new Template(app.conf.mwapi_req); // set up the RESTBase request template - if(!app.conf.restbase_req) { + if (!app.conf.restbase_req) { app.conf.restbase_req = { method: '{{request.method}}', uri: 'http://{{domain}}/api/rest_v1/{+path}', @@ -114,8 +110,8 @@ module.exports = { - mwApiGet: mwApiGet, - restApiGet: restApiGet, - setupApiTemplates: setupApiTemplates + mwApiGet, + restApiGet, + setupApiTemplates }; diff --git a/lib/swagger-ui.js b/lib/swagger-ui.js new file mode 100644 index 0000000..a85985c --- /dev/null +++ b/lib/swagger-ui.js @@ -0,0 +1,79 @@ +'use strict'; + + +const BBPromise = require('bluebird'); +const fs = BBPromise.promisifyAll(require('fs')); +const path = require('path'); +const HTTPError = require('../lib/util.js').HTTPError; + + +// Swagger-ui helpfully exporting the absolute path of its dist directory +const docRoot = `${require('swagger-ui').dist}/`; + +function processRequest(app, req, res) { + + const reqPath = req.query.path || '/index.html'; + const filePath = path.join(docRoot, reqPath); + + // Disallow relative paths. + // Test relies on docRoot ending on a slash. + if (filePath.substring(0, docRoot.length) !== docRoot) { + throw new HTTPError({ + status: 404, + type: 'not_found', + title: 'File not found', + detail: `${reqPath} could not be found.` + }); + } + + return fs.readFileAsync(filePath) + .then((body) => { + if (reqPath === '/index.html') { + body = body.toString() + .replace(/((?:src|href)=['"])/g, '$1?doc&path=') + // Some self-promotion + .replace(/<a id="logo".*?<\/a>/, + `<a id="logo" href="${app.info.homepage}">${app.info.name}</a>`) + .replace(/<title>[^<]*<\/title>/, `<title>${app.info.name}</title>`) + // Replace the default url with ours, switch off validation & + // limit the size of documents to apply syntax highlighting to + .replace(/docExpansion: "none"/, 'docExpansion: "list", ' + + 'validatorUrl: null, ' + + 'highlightSizeThreshold: 10000') + .replace(/ url: url,/, 'url: "/?spec",'); + } + + let contentType = 'text/html'; + if (/\.js$/.test(reqPath)) { + contentType = 'text/javascript'; + body = body.toString() + .replace(/underscore-min\.map/, '?doc&path=lib/underscore-min.map'); + } else if (/\.png$/.test(reqPath)) { + contentType = 'image/png'; + } else if (/\.map$/.test(reqPath)) { + contentType = 'application/json'; + } else if (/\.ttf$/.test(reqPath)) { + contentType = 'application/x-font-ttf'; + } else if (/\.css$/.test(reqPath)) { + contentType = 'text/css'; + body = body.toString().replace(/\.\.\/(images|fonts)\//g, '?doc&path=$1/'); + } + + res.setHeader('Content-Type', contentType); + res.setHeader('content-security-policy', "default-src 'none'; " + + "script-src 'self' 'unsafe-inline'; connect-src *; " + + "style-src 'self' 'unsafe-inline'; img-src 'self'; font-src 'self';"); + res.send(body.toString()); + }) + .catch({ code: 'ENOENT' }, () => { + res.status(404) + .type('not_found') + .send('not found'); + }); + +} + +module.exports = { + processRequest +}; + diff --git a/lib/util.js b/lib/util.js index 21431a3..cb3d587 100644 --- a/lib/util.js +++ b/lib/util.js @@ -1,57 +1,50 @@ 'use strict'; -var BBPromise = require('bluebird'); -var util = require('util'); -var express = require('express'); -var uuid = require('cassandra-uuid'); -var bunyan = require('bunyan'); +const BBPromise = require('bluebird'); +const express = require('express'); +const uuid = require('cassandra-uuid'); +const bunyan = require('bunyan'); /** * Error instance wrapping HTTP error responses */ -function HTTPError(response) { +class HTTPError extends Error { - Error.call(this); - Error.captureStackTrace(this, HTTPError); + constructor(response) { + super(); + Error.captureStackTrace(this, HTTPError); - if(response.constructor !== Object) { - // just assume this is just the error message - var msg = response; - response = { - status: 500, - type: 'internal_error', - title: 'InternalError', - detail: msg - }; + if (response.constructor !== Object) { + // just assume this is just the error message + response = { + status: 500, + type: 'internal_error', + title: 'InternalError', + detail: response + }; + } + + this.name = this.constructor.name; + this.message = `${response.status}`; + if (response.type) { + this.message += `: ${response.type}`; + } + + Object.assign(this, response); } - - this.name = this.constructor.name; - this.message = response.status + ''; - if(response.type) { - this.message += ': ' + response.type; - } - - for (var key in response) { - this[key] = response[key]; - } - } - -util.inherits(HTTPError, Error); - /** * Generates an object suitable for logging out of a request object - * - * @param {Request} req the request - * @param {RegExp} whitelistRE the RegExp used to filter headers - * @return {Object} an object containing the key components of the request + * @param {!Request} req the request + * @param {?RegExp} whitelistRE the RegExp used to filter headers + * @return {!Object} an object containing the key components of the request */ function reqForLog(req, whitelistRE) { - var ret = { + const ret = { url: req.originalUrl, headers: {}, method: req.method, @@ -62,9 +55,9 @@ remotePort: req.connection.remotePort }; - if(req.headers && whitelistRE) { - Object.keys(req.headers).forEach(function(hdr) { - if(whitelistRE.test(hdr)) { + if (req.headers && whitelistRE) { + Object.keys(req.headers).forEach((hdr) => { + if (whitelistRE.test(hdr)) { ret.headers[hdr] = req.headers[hdr]; } }); @@ -76,20 +69,19 @@ /** - * Serialises an error object in a form suitable for logging - * - * @param {Error} err error to serialise - * @return {Object} the serialised version of the error + * Serialises an error object in a form suitable for logging. + * @param {!Error} err error to serialise + * @return {!Object} the serialised version of the error */ function errForLog(err) { - var ret = bunyan.stdSerializers.err(err); + const ret = bunyan.stdSerializers.err(err); ret.status = err.status; ret.type = err.type; ret.detail = err.detail; // log the stack trace only for 500 errors - if(Number.parseInt(ret.status) !== 500) { + if (Number.parseInt(ret.status, 10) !== 500) { ret.stack = undefined; } @@ -98,9 +90,8 @@ } /** - * Generates a unique request ID - * - * @return {String} the generated request ID + * Generates a unique request ID. + * @return {!String} the generated request ID */ function generateRequestId() { @@ -114,37 +105,34 @@ * promised try blocks so as to allow catching all errors, * regardless of whether a handler returns/uses promises * or not. - * - * @param {Object} route the object containing the router and path to bind it to - * @param {Application} app the application object + * @param {!Object} route the object containing the router and path to bind it to + * @param {!Application} app the application object */ function wrapRouteHandlers(route, app) { - route.router.stack.forEach(function(routerLayer) { - var path = (route.path + routerLayer.route.path.slice(1)) + route.router.stack.forEach((routerLayer) => { + let path = (route.path + routerLayer.route.path.slice(1)) .replace(/\/:/g, '/--') .replace(/^\//, '') - .replace(/[\/?]+$/, ''); + .replace(/[/?]+$/, ''); path = app.metrics.normalizeName(path || 'root'); - routerLayer.route.stack.forEach(function(layer) { - var origHandler = layer.handle; + routerLayer.route.stack.forEach((layer) => { + const origHandler = layer.handle; layer.handle = function(req, res, next) { - var startTime = Date.now(); - BBPromise.try(function() { - return origHandler(req, res, next); - }) + const startTime = Date.now(); + BBPromise.try(() => origHandler(req, res, next)) .catch(next) - .finally(function() { - var statusCode = parseInt(res.statusCode) || 500; - if(statusCode < 100 || statusCode > 599) { + .finally(() => { + let statusCode = parseInt(res.statusCode, 10) || 500; + if (statusCode < 100 || statusCode > 599) { statusCode = 500; } - var statusClass = Math.floor(statusCode / 100) + 'xx'; - var stat = path + '.' + req.method + '.'; + const statusClass = `${Math.floor(statusCode / 100)}xx`; + const stat = `${path}.${req.method}.`; app.metrics.endTiming([ stat + statusCode, stat + statusClass, - stat + 'ALL' + `${stat}ALL` ], startTime); }); }; @@ -155,24 +143,22 @@ /** - * Generates an error handler for the given applications - * and installs it. Usage: - * - * @param {Application} app the application object to add the handler to + * Generates an error handler for the given applications and installs it. + * @param {!Application} app the application object to add the handler to */ function setErrorHandler(app) { - app.use(function(err, req, res, next) { - var errObj; + app.use((err, req, res, next) => { + let errObj; // ensure this is an HTTPError object - if(err.constructor === HTTPError) { + if (err.constructor === HTTPError) { errObj = err; - } else if(err instanceof Error) { + } else if (err instanceof Error) { // is this an HTTPError defined elsewhere? (preq) - if(err.constructor.name === 'HTTPError') { - var o = { status: err.status }; - if(err.body && err.body.constructor === Object) { - Object.keys(err.body).forEach(function(key) { + if (err.constructor.name === 'HTTPError') { + const o = { status: err.status }; + if (err.body && err.body.constructor === Object) { + Object.keys(err.body).forEach((key) => { o[key] = err.body[key]; }); } else { @@ -190,7 +176,7 @@ stack: err.stack }); } - } else if(err.constructor === Object) { + } else if (err.constructor === Object) { // this is a regular object, suppose it's a response errObj = new HTTPError(err); } else { @@ -203,26 +189,25 @@ }); } // ensure some important error fields are present - if(!errObj.status) { errObj.status = 500; } - if(!errObj.type) { errObj.type = 'internal_error'; } + if (!errObj.status) { errObj.status = 500; } + if (!errObj.type) { errObj.type = 'internal_error'; } // add the offending URI and method as well - if(!errObj.method) { errObj.method = req.method; } - if(!errObj.uri) { errObj.uri = req.url; } + if (!errObj.method) { errObj.method = req.method; } + if (!errObj.uri) { errObj.uri = req.url; } // some set 'message' or 'description' instead of 'detail' errObj.detail = errObj.detail || errObj.message || errObj.description || ''; // adjust the log level based on the status code - var level = 'error'; - if(Number.parseInt(errObj.status) < 400) { + let level = 'error'; + if (Number.parseInt(errObj.status, 10) < 400) { level = 'trace'; - } else if(Number.parseInt(errObj.status) < 500) { + } else if (Number.parseInt(errObj.status, 10) < 500) { level = 'info'; } // log the error - (req.logger || app.logger).log(level + '/' + - (errObj.component ? errObj.component : errObj.status), - errForLog(errObj)); + const component = (errObj.component ? errObj.component : errObj.status); + (req.logger || app.logger).log(`${level}/${component}`, errForLog(errObj)); // let through only non-sensitive info - var respBody = { + const respBody = { status: errObj.status, type: errObj.type, title: errObj.title, @@ -238,46 +223,45 @@ /** * Creates a new router with some default options. - * - * @param {Object} [opts] additional options to pass to express.Router() - * @return {Router} a new router object + * @param {?Object} [opts] additional options to pass to express.Router() + * @return {!Router} a new router object */ function createRouter(opts) { - var options = { + const options = { mergeParams: true }; - if(opts && opts.constructor === Object) { - Object.keys(opts).forEach(function(key) { - options[key] = opts[key]; - }); + if (opts && opts.constructor === Object) { + Object.assign(options, opts); } - return express.Router(options); + return new express.Router(options); } /** - * Adds logger to the request and logs it - * - * @param {*} req request object - * @param {Application} app application object + * Adds logger to the request and logs it. + * @param {!*} req request object + * @param {!Application} app application object */ function initAndLogRequest(req, app) { req.headers = req.headers || {}; req.headers['x-request-id'] = req.headers['x-request-id'] || generateRequestId(); - req.logger = app.logger.child({request_id: req.headers['x-request-id'], request: reqForLog(req, app.conf.log_header_whitelist)}); - req.logger.log('trace/req', {msg: 'incoming request'}); + req.logger = app.logger.child({ + request_id: req.headers['x-request-id'], + request: reqForLog(req, app.conf.log_header_whitelist) + }); + req.logger.log('trace/req', { msg: 'incoming request' }); } module.exports = { - HTTPError: HTTPError, - initAndLogRequest: initAndLogRequest, - wrapRouteHandlers: wrapRouteHandlers, - setErrorHandler: setErrorHandler, + HTTPError, + initAndLogRequest, + wrapRouteHandlers, + setErrorHandler, router: createRouter }; diff --git a/lib/vega.js b/lib/vega.js index a8423db..a948507 100644 --- a/lib/vega.js +++ b/lib/vega.js @@ -1,19 +1,20 @@ 'use strict'; + /* global module */ -var _ = require('underscore'); -var BBPromise = require('bluebird'); -var vega1 = require('./vega1'); -var vega2 = require('./vega2'); +const _ = require('underscore'); +const BBPromise = require('bluebird'); +const vega1 = require('./vega1'); +const vega2 = require('./vega2'); module.exports = {}; /** * Init vega rendering - * @param log - * @param conf - object with configuration parameters + * @param {Function} log + * @param {Object} conf - object with configuration parameters */ -module.exports.initVega = function (log, conf) { +module.exports.initVega = function(log, conf) { if (!module.exports._initialized) { module.exports._initialized = true; vega1.initVega(log); @@ -23,24 +24,25 @@ /** * - * @param domain domain context of the request - * @param spec graph specification - * @param format string format - png or svg - * @param response object to send/stream the results to. If not given, returns [value, isLegacy] + * @param {string} domain domain context of the request + * @param {Object} spec graph specification + * @param {string} format string format - png or svg + * @param {Response} response object to send/stream the results to. + * If not given, returns [value, isLegacy] * @param {Object} headers sets response headers like 'Cache-Control' - * @returns {*} promise + * @return {*} promise */ module.exports.render = function(domain, spec, format, response, headers) { - var isSvg = format === 'svg', - resultVersion; + const isSvg = format === 'svg'; + let resultVersion; - var p = BBPromise.try(function () { + const p = BBPromise.try(() => { if (typeof spec === 'string') { spec = JSON.parse(spec); } // the spec may contain vega version - var version = _.has(spec, 'version') ? spec.version : 2; - resultVersion = 'v' + version; + const version = _.has(spec, 'version') ? spec.version : 2; + resultVersion = `v${version}`; switch (version) { case 1: @@ -52,7 +54,7 @@ } }); - return p.then(function (result) { + return p.then((result) => { if (!response) { return [result, resultVersion]; } @@ -67,16 +69,16 @@ } // PNG stream copy - return new BBPromise(function (resolve, reject) { - var stream = result.pngStream(); - stream.on('data', function (chunk) { + return new BBPromise(((resolve, reject) => { + const stream = result.pngStream(); + stream.on('data', (chunk) => { response.write(chunk); }); stream.on('error', reject); - stream.on('end', function () { + stream.on('end', () => { response.end(); resolve(resultVersion); }); - }); + })); }); }; diff --git a/lib/vega1.js b/lib/vega1.js index 5bc2028..8015c5e 100644 --- a/lib/vega1.js +++ b/lib/vega1.js @@ -1,27 +1,29 @@ 'use strict'; + /* global module */ -var BBPromise = require('bluebird'); -var vega1x = require('vega-1x'); +const BBPromise = require('bluebird'); +const vega1x = require('vega-1x'); -var canvas1ToBuffer; -var vega1xRenderAsync = BBPromise.promisify(vega1x.headless.render, vega1x.headless); +let canvas1ToBuffer; +const vega1xRenderAsync = BBPromise.promisify(vega1x.headless.render, vega1x.headless); module.exports = {}; /** * Init vega rendering - * @param log + * @param {Function} log */ -module.exports.initVega = function (log) { +module.exports.initVega = function(log) { vega1x.config.safeMode = true; - vega1x.config.isNode = true; // Vega is flaky with its own detection, fails in tests and with IDE debug + // Vega is flaky with its own detection, fails in tests and with IDE debug + vega1x.config.isNode = true; // set up vega loggers to log to our device instead of stderr - vega1x.log = function (msg) { + vega1x.log = function(msg) { log('debug/vega', msg); }; - vega1x.error = function (msg) { + vega1x.error = function(msg) { log('warn/vega', msg); }; @@ -35,11 +37,11 @@ }; }; -module.exports.render = function (spec, canvasToBuffer) { - var p = vega1xRenderAsync({spec: spec, renderer: 'canvas'}).get('canvas'); +module.exports.render = function(spec, canvasToBuffer) { + let p = vega1xRenderAsync({ spec, renderer: 'canvas' }).get('canvas'); if (canvasToBuffer) { // Convert canvas content to a buffer - p = p.then(function(result) { + p = p.then((result) => { if (!canvas1ToBuffer) { canvas1ToBuffer = BBPromise.promisify(result.toBuffer); } diff --git a/lib/vega2.js b/lib/vega2.js index 7015fbf..c83363e 100644 --- a/lib/vega2.js +++ b/lib/vega2.js @@ -1,13 +1,14 @@ 'use strict'; + /* global module */ -let _ = require('underscore'), - urllib = require('url'), - BBPromise = require('bluebird'), - vega = require('vega'), - VegaWrapper = require('mw-graph-shared'), - vegaSpecParseAsync = BBPromise.promisify(vega.parse.spec, {context: vega.parse}), - canvas2ToBuffer; +const urllib = require('url'); +const BBPromise = require('bluebird'); +const vega = require('vega'); +const VegaWrapper = require('mw-graph-shared'); +const vegaSpecParseAsync = BBPromise.promisify(vega.parse.spec, { context: vega.parse }); + +let canvas2ToBuffer; module.exports = {}; @@ -18,30 +19,31 @@ * @param {Object} conf.allowedDomains * @param {Object} conf.domainMap */ -module.exports.initVega = function (log, conf) { - let domains = conf.allowedDomains; +module.exports.initVega = function(log, conf) { + const domains = conf.allowedDomains; if (typeof domains !== 'object' || Object.keys(domains).length === 0) { - log('fatal/config', 'Config must have a list of valid domains in the allowedDomains object'); + log('fatal/config', + 'Config must have a list of valid domains in the allowedDomains object'); process.exit(1); } // set up vega loggers to log to our device instead of stderr - vega.logging.log = function (msg) { + vega.logging.log = function(msg) { log('debug/vega', msg); }; - vega.logging.error = function (msg) { + vega.logging.error = function(msg) { log('error/vega', msg); - //throw new Error(msg); + // throw new Error(msg); }; - let parseUrl = function (opt) { + function parseUrl(opt) { let url = opt.url; - let isRelativeUrl = url[0] === '/' && url[1] === '/'; + const isRelativeUrl = url[0] === '/' && url[1] === '/'; if (isRelativeUrl) { // Workaround: urllib does not support relative URLs, add a temp protocol - url = 'temp:' + url; + url = `temp:${url}`; } - let urlParts = urllib.parse(url, true); + const urlParts = urllib.parse(url, true); if (isRelativeUrl) { delete urlParts.protocol; } @@ -57,24 +59,26 @@ // this value is ignored by the urllib.format() urlParts.isRelativeHost = true; } - // Temporary workaround until we know the needed language for sure (probably as a graphoid URL parameter) - // Only take the first portion of the domain if it is 2-3 letters, or if there is a '-' afterwards. + // Temporary workaround until we know the needed language for sure + // (probably as a graphoid URL parameter) + // Only take the first portion of the domain if it is 2-3 letters, + // or if there is a '-' afterwards. // urllib.format will ignore this field. - let site = /^([a-z]{2,3}(-[-a-z]+)?)\./i.exec(opt.domain); + const site = /^([a-z]{2,3}(-[-a-z]+)?)\./i.exec(opt.domain); if (site) { urlParts.siteLanguage = site[1]; } return urlParts; - }; + } module.exports.sharedLib = new VegaWrapper({ datalib: vega.util, useXhr: false, isTrusted: false, - domains: domains, + domains, domainMap: conf.domainMap, logger: vega.logging.log, - parseUrl: parseUrl, + parseUrl, formatUrl: urllib.format }); @@ -83,33 +87,33 @@ /** * - * @param domain domain context of the request - * @param spec graph specification + * @param {string} domain domain context of the request + * @param {Object} spec graph specification * @param {boolean} isSvg png or svg * @param {boolean} canvasToBuffer when true, converts canvas to a buffer - * @returns {*} promise + * @return {*} promise */ -module.exports.render = function (domain, spec, isSvg, canvasToBuffer) { - let p = BBPromise.try(function () { - let config = { +module.exports.render = function(domain, spec, isSvg, canvasToBuffer) { + let p = BBPromise.try(() => { + const config = { load: { - domain: domain + domain } }; - return vegaSpecParseAsync(spec, config).then(function (chart) { - let view = chart({renderer: (isSvg ? 'svg' : 'canvas')}).update(); + return vegaSpecParseAsync(spec, config).then((chart) => { + const view = chart({ renderer: (isSvg ? 'svg' : 'canvas') }).update(); if (isSvg) { return view.svg(); } - return new BBPromise(function (resolve) { + return new BBPromise(((resolve) => { view.canvasAsync(resolve); - }); + })); }); }); if (canvasToBuffer) { // Convert canvas content to a buffer - p = p.then(function(result) { + p = p.then((result) => { if (!canvas2ToBuffer) { canvas2ToBuffer = BBPromise.promisify(result.toBuffer); } diff --git a/package.json b/package.json index e911728..d0531c3 100644 --- a/package.json +++ b/package.json @@ -1,13 +1,15 @@ { "name": "graphoid", - "version": "0.1.14", + "version": "0.1.15", "description": "Renders vega graphs from mediawiki pages", "main": "./app.js", "scripts": { "start": "service-runner", - "test": "mocha && nsp check", + "test": "PREQ_CONNECT_TIMEOUT=15 mocha && nsp check", + "lint": "eslint --cache --max-warnings 0 --ext .js --ext .json .", "docker-start": "service-runner docker-start", "docker-test": "service-runner docker-test", + "test-build": "service-runner docker-test && service-runner build --deploy-repo --force", "coverage": "istanbul cover _mocha -- -R spec" }, "repository": { @@ -27,33 +29,40 @@ "bugs": "https://phabricator.wikimedia.org/tag/service-template-node/", "homepage": "https://www.mediawiki.org/wiki/Extension:Graph", "dependencies": { - "bluebird": "^3.4.1", - "body-parser": "^1.15.2", - "bunyan": "^1.8.1", + "bluebird": "^3.5.1", + "body-parser": "^1.18.2", + "bunyan": "^1.8.12", "cassandra-uuid": "^0.0.2", - "compression": "^1.6.2", "core-js": "^2.4.1", - "domino": "^1.0.25", - "express": "^4.14.0", - "js-yaml": "^3.6.1", - "preq": "^0.5.2", - "service-runner": "^2.2.5", - "swagger-router": "^0.4.6" - - , + "compression": "^1.7.1", + "domino": "^1.0.30", + "express": "^4.16.2", + "js-yaml": "^3.10.0", + "preq": "^0.5.3", + "service-runner": "^2.4.2", + "swagger-router": "^0.7.1", + "swagger-ui": "git+https://github.com/wikimedia/swagger-ui#master", + "http-shutdown": "^1.2.0", "underscore": "^1.8.3", "vega-1x": "git+http://g...@github.com/nyurik/vega-1x", "mw-graph-shared": "^0.3.6", "vega": "git+http://g...@github.com/nyurik/vega" }, "devDependencies": { + "ajv": "^5.5.0", "bunyan-prettystream": "*", - "extend": "^3.0.0", - "istanbul": "^0.4.4", - "mocha": "^2.5.3", + "extend": "^3.0.1", + "istanbul": "^0.4.5", + "mocha": "^4.0.1", "mocha-jshint": "^2.3.1", - "mocha-lcov-reporter": "^1.2.0", - "nsp": "^2.6.1" + "mocha-lcov-reporter": "^1.3.0", + "nsp": "^2.8.1", + "mocha-eslint": "^4.1.0", + "eslint": "^4.12.0", + "eslint-config-node-services": "^2.2.5", + "eslint-config-wikimedia": "^0.5.0", + "eslint-plugin-json": "^1.2.0", + "eslint-plugin-jsdoc": "^3.0.0" }, "deploy": { "target": "debian", diff --git a/routes/graphoid-v1.js b/routes/graphoid-v1.js index e9933f0..f81b709 100644 --- a/routes/graphoid-v1.js +++ b/routes/graphoid-v1.js @@ -1,37 +1,37 @@ 'use strict'; -var Promise = require('bluebird'); -var preq = require('preq'); -var sUtil = require('../lib/util'); -var vega = require('../lib/vega'); +const BBPromise = require('bluebird'); +const preq = require('preq'); +const sUtil = require('../lib/util'); +const vega = require('../lib/vega'); /** * Main log function */ -var log; +let log; /** * Metrics object */ -var metrics; +let metrics; /** * Limit request to 10 seconds by default */ -var timeout = 10000; +let timeout = 10000; /** * Cache header to set on the responses * @type {Object} */ -var headers; +let headers; /** * Cache header to set on the responses * @type {Object} */ -var errorHeaders; +let errorHeaders; /* @@ -47,11 +47,11 @@ // NOTE: there are a few libraries that do this function merge() { - var result = {}, - args = Array.prototype.slice.apply(arguments); + const result = {}; + const args = Array.prototype.slice.apply(arguments); - args.forEach(function (arg) { - Object.getOwnPropertyNames(arg).forEach(function (prop) { + args.forEach((arg) => { + Object.getOwnPropertyNames(arg).forEach((prop) => { result[prop] = arg[prop]; }); }); @@ -61,16 +61,15 @@ // Adapted from https://www.promisejs.org/patterns/ function delay(time) { - return new Promise(function (fulfill) { + return new BBPromise(((fulfill) => { setTimeout(fulfill, time); - }); + })); } function failOnTimeout(promise, time) { - return time <= 0 ? promise : - Promise.race([promise, delay(time).then(function () { - throw 'timeout'; // we later compare on this value - })]); + return time <= 0 ? promise : BBPromise.race([promise, delay(time).then(() => { + throw new Error('timeout'); // we later compare on this value + })]); } /** @@ -78,15 +77,15 @@ */ function validateRequest(state) { - var start = Date.now(); + const start = Date.now(); - var p = state.request.params, - format = p.format, - domain = p.domain, - title = p.title, - id_ext = p.id.split('.', 2), - id = id_ext[0], - ext = id_ext[1]; + const p = state.request.params; + const format = p.format; + const title = p.title; + const idExt = p.id.split('.', 2); + const id = idExt[0]; + const ext = idExt[1]; + let domain = p.domain; state.log = p; // log all parameters of the request @@ -118,7 +117,7 @@ } state.apiRequest.hash = id; - var sanitizedHost = vega.lib.sharedLib.sanitizeHost(domain); + const sanitizedHost = vega.lib.sharedLib.sanitizeHost(domain); if (!sanitizedHost) { throw new Err('info/param-domain', 'req.domain'); } @@ -127,43 +126,45 @@ state.log.backend = domain; } state.domain = domain; - state.apiUrl = sanitizedHost.protocol + '//' + domain + '/w/api.php'; + state.apiUrl = `${sanitizedHost.protocol}//${domain}/w/api.php`; // Log which wiki is actually requesting this if (domain.endsWith('.org')) { domain = domain.substr(0, domain.length - 4); } - metrics.endTiming('total.req.' + domain.replace('.', '-'), start); + metrics.endTiming(`total.req.${domain.replace('.', '-')}`, start); return state; } /** * Retrieve graph specifications from the domain - * @param state is the object with the current state of the request processing + * @param {Object} state is the object with the current state of the request processing */ function downloadGraphDef(state) { - var startDefDownload = Date.now(); + const startDefDownload = Date.now(); state.log.apicall = state.apiRequest; - var requestOpts = { + const requestOpts = { uri: state.apiUrl, query: state.apiRequest, - headers: {'User-Agent': 'graphoid (yurik at wikimedia)'} + headers: { 'User-Agent': 'graphoid (yurik at wikimedia)' } }; - return preq(requestOpts).then(function (apiRes) { + return preq(requestOpts).then((apiRes) => { if (apiRes.status !== 200) { state.log.apiRetStatus = apiRes.status; throw new Err('error/mwapi-status', 'mwapi.bad-status'); } - var res = apiRes.body; + const res = apiRes.body; + // eslint-disable-next-line no-prototype-builtins if (res.hasOwnProperty('error')) { state.log.apiRetError = res.error; throw new Err('info/mwapi-error', 'mwapi.error'); } + // eslint-disable-next-line no-prototype-builtins if (res.hasOwnProperty('warnings')) { state.log.apiWarning = res.warnings; state.request.logger.log('info/mwapi-warning', state.log); @@ -177,13 +178,13 @@ } function renderOnCanvas(state) { - var start = Date.now(); + const start = Date.now(); return vega .render(state.domain, state.graphData, 'png', state.response, headers) - .then(function () { + .then(() => { metrics.endTiming('total.vega', start); }) - .catch(function (err) { + .catch((err) => { state.log.vegaErr = err.stack; throw new Err('error/vega', 'vega.error'); }); @@ -194,29 +195,29 @@ */ function renderByHash(req, res) { - var start = Date.now(); - var state = {request: req, response: res}; + const start = Date.now(); + const state = { request: req, response: res }; - var render = Promise + const render = BBPromise .resolve(state) .then(validateRequest) .then(downloadGraphDef) .then(renderOnCanvas); return failOnTimeout(render, timeout) - .then(function () { + .then(() => { // SUCCESS // For now, record everything, but soon we should scale it back req.logger.log('trace/ok', state.log); metrics.endTiming('total.success', start); - }, function (reason) { + }, (reason) => { // FAILURE - var l = state.log; - var msg = 'error/unknown', - mx = 'error.unknown'; + let l = state.log; + let msg = 'error/unknown'; + let mx = 'error.unknown'; if (reason instanceof Err) { l = merge(reason, l); @@ -243,14 +244,14 @@ * Initialize a route * @param {Object} app * @param {Object} app.logger - * @param {function} app.logger.log + * @param {Function} app.logger.log * @param {Object} app.conf * @param {number} app.conf.timeout * @param {Object} app.conf.headers * @param {Object} app.conf.errorHeaders * @param {Object} app.metrics - * @param {function} app.metrics.increment - * @returns {{path: string, api_version: number, router: Router}} + * @param {Function} app.metrics.increment + * @return {{path: string, api_version: number, router: Router}} */ module.exports = function(app) { @@ -261,19 +262,19 @@ log('info/init', 'starting v1'); metrics.increment('v1.init'); - var conf = app.conf; + const conf = app.conf; timeout = conf.timeout || timeout; - headers = conf.headers || {'Cache-Control': 'public, s-maxage=300, max-age=300'}; + headers = conf.headers || { 'Cache-Control': 'public, s-maxage=300, max-age=300' }; errorHeaders = conf.errorHeaders || headers; vega.initVega(log, conf); - var router = sUtil.router(); + const router = sUtil.router(); router.get('/:format/:title/:revid/:id', renderByHash); return { path: '/', api_version: 1, - router: router + router }; }; diff --git a/routes/graphoid-v2.js b/routes/graphoid-v2.js index f01fe77..99ce04d 100644 --- a/routes/graphoid-v2.js +++ b/routes/graphoid-v2.js @@ -1,38 +1,37 @@ 'use strict'; -var _ = require('underscore'); -var Promise = require('bluebird'); -var preq = require('preq'); -var sUtil = require('../lib/util'); -var vega = require('../lib/vega'); +const _ = require('underscore'); +const BBPromise = require('bluebird'); +const sUtil = require('../lib/util'); +const vega = require('../lib/vega'); /** * Main log function */ -var log; +let log; /** * Metrics object */ -var metrics; +let metrics; /** * Limit request to 10 seconds by default */ -var timeout = 10000; +let timeout = 10000; /** * Cache header to set on the responses * @type {Object} */ -var headers; +let headers; /** * Cache header to set on the responses * @type {Object} */ -var errorHeaders; +let errorHeaders; /* * Utility functions @@ -47,16 +46,15 @@ // Adapted from https://www.promisejs.org/patterns/ function delay(time) { - return new Promise(function (fulfill) { + return new BBPromise(((fulfill) => { setTimeout(fulfill, time); - }); + })); } function failOnTimeout(promise, time) { - return time <= 0 ? promise : - Promise.race([promise, delay(time).then(function () { - throw 'timeout'; // we later compare on this value - })]); + return time <= 0 ? promise : BBPromise.race([promise, delay(time).then(() => { + throw new Error('timeout'); // we later compare on this value + })]); } /** @@ -64,10 +62,10 @@ */ function validateRequest(state) { - var p = state.request.params, - format = p.format, - domain = p.domain, - body = state.request.body; + const p = state.request.params; + const format = p.format; + const body = state.request.body; + let domain = p.domain; state.log = p; // log all parameters of the request @@ -76,7 +74,7 @@ } state.format = format; - var sanitizedHost = vega.lib.sharedLib.sanitizeHost(domain); + const sanitizedHost = vega.lib.sharedLib.sanitizeHost(domain); if (!sanitizedHost) { throw new Err('info/param-domain', 'req.domain'); } @@ -95,19 +93,19 @@ if (domain.endsWith('.org')) { domain = domain.substr(0, domain.length - 4); } - metrics.increment('req.' + domain.replace('.', '-')); + metrics.increment(`req.${domain.replace('.', '-')}`); return state; } /** - * - * @param {String} state.request.headers.title - * @param {String} state.request.headers.revisionid - * @returns {*|{value, done}} + * @param {Object} state + * @param {string} state.request.headers.title + * @param {string} state.request.headers.revisionid + * @return {*|{value, done}} */ function renderRequest(state) { - var start = Date.now(); + const start = Date.now(); // headers always received in lower case if (state.request.headers.title) { state.response.header('Title', state.request.headers.title); @@ -117,9 +115,9 @@ } return vega .render(state.domain, state.graphData, state.format, state.response, headers) - .then(function () { + .then(() => { metrics.endTiming('total.vega', start); - }).catch(function (err) { + }).catch((err) => { state.log.vegaErr = err.message; state.log.vegaErrStack = err.stack; throw new Err('error/vega', 'vega.error'); @@ -131,26 +129,26 @@ */ function renderGraph(req, res) { - var start = Date.now(); - var state = {request: req, response: res}; + const start = Date.now(); + const state = { request: req, response: res }; - var render = Promise + const render = BBPromise .resolve(state) .then(validateRequest) .then(renderRequest); return failOnTimeout(render, timeout) - .then(function () { + .then(() => { // SUCCESS metrics.endTiming('total.success', start); - }, function (reason) { + }, (reason) => { // FAILURE - var l = state.log; - var msg = 'error/unknown', - mx = 'error.unknown'; + let l = state.log; + let msg = 'error/unknown'; + let mx = 'error.unknown'; if (reason instanceof Err) { l = _.extend(reason, l); @@ -177,14 +175,14 @@ * Initialize a route * @param {Object} app * @param {Object} app.logger - * @param {function} app.logger.log + * @param {Function} app.logger.log * @param {Object} app.conf * @param {number} app.conf.timeout * @param {Object} app.conf.headers * @param {Object} app.conf.errorHeaders * @param {Object} app.metrics - * @param {function} app.metrics.increment - * @returns {{path: string, api_version: number, router: Router}} + * @param {Function} app.metrics.increment + * @return {{path: string, api_version: number, router: Router}} */ module.exports = function(app) { @@ -195,21 +193,21 @@ log('info/init', 'starting v2'); metrics.increment('v2.init'); - var conf = app.conf; + const conf = app.conf; timeout = conf.timeout || timeout; - headers = conf.headers || {'Cache-Control': 'public, s-maxage=300, max-age=300'}; + headers = conf.headers || { 'Cache-Control': 'public, s-maxage=300, max-age=300' }; errorHeaders = conf.errorHeaders || headers; vega.initVega(log, conf); - var router = sUtil.router(); - //var bodyParser = require('body-parser').json(); + const router = sUtil.router(); + // var bodyParser = require('body-parser').json(); router.post('/:format', renderGraph); return { path: '/', api_version: 2, - router: router + router }; }; diff --git a/routes/info.js b/routes/info.js index 5dd2590..832af0d 100644 --- a/routes/info.js +++ b/routes/info.js @@ -1,25 +1,25 @@ 'use strict'; -var sUtil = require('../lib/util'); +const sUtil = require('../lib/util'); /** * The main router object */ -var router = sUtil.router(); +const router = sUtil.router(); /** * The main application object reported when this module is require()d */ -var app; +let app; /** * GET / * Gets some basic info about this service */ -router.get('/', function(req, res) { +router.get('/', (req, res) => { // simple sync return res.json({ @@ -36,7 +36,7 @@ * GET /name * Gets the service's name as defined in package.json */ -router.get('/name', function(req, res) { +router.get('/name', (req, res) => { // simple return res.json({ name: app.info.name }); @@ -48,7 +48,7 @@ * GET /version * Gets the service's version as defined in package.json */ -router.get('/version', function(req, res) { +router.get('/version', (req, res) => { // simple return res.json({ version: app.info.version }); @@ -61,29 +61,28 @@ * Redirects to the service's home page if one is given, * returns a 404 otherwise */ -router.all('/home', function(req, res) { +router.all('/home', (req, res) => { - var home = app.info.homepage; - if(home && /^http/.test(home)) { + const home = app.info.homepage; + if (home && /^http/.test(home)) { // we have a home page URI defined, so send it res.redirect(301, home); - return; } else { // no URI defined for the home page, error out - res.status(404).end('No home page URL defined for ' + app.info.name); + res.status(404).end(`No home page URL defined for ${app.info.name}`); } }); -module.exports = function(appObj) { +module.exports = (appObj) => { app = appObj; return { path: '/_info', skip_domain: true, - router: router + router }; }; diff --git a/routes/root.js b/routes/root.js index 3b34d8e..83ee521 100644 --- a/routes/root.js +++ b/routes/root.js @@ -1,25 +1,26 @@ 'use strict'; -var sUtil = require('../lib/util'); +const sUtil = require('../lib/util'); +const swaggerUi = require('../lib/swagger-ui'); /** * The main router object */ -var router = sUtil.router(); +const router = sUtil.router(); /** * The main application object reported when this module is require()d */ -var app; +let app; /** * GET /robots.txt * Instructs robots no indexing should occur on this domain. */ -router.get('/robots.txt', function(req, res) { +router.get('/robots.txt', (req, res) => { res.set({ 'User-agent': '*', @@ -31,28 +32,30 @@ /** * GET / - * Main entry point. Currently it only responds if the spec query + * Main entry point. Currently it only responds if the spec or doc query * parameter is given, otherwise lets the next middleware handle it */ -router.get('/', function(req, res, next) { +router.get('/', (req, res, next) => { - if(!(req.query || {}).hasOwnProperty('spec')) { - next(); - } else { + if ({}.hasOwnProperty.call(req.query || {}, 'spec')) { res.json(app.conf.spec); + } else if ({}.hasOwnProperty.call(req.query || {}, 'doc')) { + return swaggerUi.processRequest(app, req, res); + } else { + next(); } }); -module.exports = function(appObj) { +module.exports = (appObj) => { app = appObj; return { path: '/', skip_domain: true, - router: router + router }; }; diff --git a/server.js b/server.js index a45b9ce..2a5ac56 100755 --- a/server.js +++ b/server.js @@ -8,5 +8,5 @@ // (config.yaml by default, specify other path with -c). It requires the // module(s) specified in the config 'services' section (app.js in this // example). -var ServiceRunner = require('service-runner'); +const ServiceRunner = require('service-runner'); new ServiceRunner().start(); diff --git a/test/features/app/app.js b/test/features/app/app.js index d30b94e..70755da 100644 --- a/test/features/app/app.js +++ b/test/features/app/app.js @@ -1,33 +1,39 @@ +/* global describe, it, before, after */ + 'use strict'; -var preq = require('preq'); -var assert = require('../../utils/assert.js'); -var server = require('../../utils/server.js'); +const preq = require('preq'); +const assert = require('../../utils/assert.js'); +const server = require('../../utils/server.js'); +if (!server.stopHookAdded) { + server.stopHookAdded = true; + after(() => server.stop()); +} describe('express app', function() { - this.timeout(20000); + this.timeout(20000); // eslint-disable-line no-invalid-this - before(function () { return server.start(); }); + before(() => { return server.start(); }); - it('should get robots.txt', function() { + it('should get robots.txt', () => { return preq.get({ - uri: server.config.uri + 'robots.txt' - }).then(function(res) { + uri: `${server.config.uri}robots.txt` + }).then((res) => { assert.deepEqual(res.status, 200); - assert.deepEqual(res.headers['disallow'], '/'); + assert.deepEqual(res.headers.disallow, '/'); }); }); - it('should set CORS headers', function() { - if(server.config.service.conf.cors === false) { + it('should set CORS headers', () => { + if (server.config.service.conf.cors === false) { return true; } return preq.get({ - uri: server.config.uri + 'robots.txt' - }).then(function(res) { + uri: `${server.config.uri}robots.txt` + }).then((res) => { assert.deepEqual(res.status, 200); assert.deepEqual(res.headers['access-control-allow-origin'], '*'); assert.deepEqual(!!res.headers['access-control-allow-headers'], true); @@ -35,13 +41,13 @@ }); }); - it('should set CSP headers', function() { - if(server.config.service.conf.csp === false) { + it('should set CSP headers', () => { + if (server.config.service.conf.csp === false) { return true; } return preq.get({ - uri: server.config.uri + 'robots.txt' - }).then(function(res) { + uri: `${server.config.uri}robots.txt` + }).then((res) => { assert.deepEqual(res.status, 200); assert.deepEqual(res.headers['x-xss-protection'], '1; mode=block'); assert.deepEqual(res.headers['x-content-type-options'], 'nosniff'); @@ -52,29 +58,29 @@ }); }); - it.skip('should get static content gzipped', function() { + it.skip('should get static content gzipped', () => { return preq.get({ - uri: server.config.uri + 'static/index.html', + uri: `${server.config.uri}static/index.html`, headers: { 'accept-encoding': 'gzip, deflate' } - }).then(function(res) { + }).then((res) => { // check that the response is gzip-ed assert.deepEqual(res.headers['content-encoding'], 'gzip', 'Expected gzipped contents!'); }); }); - it('should get static content uncompressed', function() { + it('should get static content uncompressed', () => { return preq.get({ - uri: server.config.uri + 'static/index.html', + uri: `${server.config.uri}static/index.html`, headers: { 'accept-encoding': '' } - }).then(function(res) { + }).then((res) => { // check that the response is gzip-ed - assert.deepEqual(res.headers['content-encoding'], undefined, 'Did not expect gzipped contents!'); + const contentEncoding = res.headers['content-encoding']; + assert.deepEqual(contentEncoding, undefined, 'Did not expect gzipped contents!'); }); }); - }); diff --git a/test/features/app/spec.js b/test/features/app/spec.js index 570fb40..eb80baa 100644 --- a/test/features/app/spec.js +++ b/test/features/app/spec.js @@ -1,25 +1,31 @@ +/* global describe, it, before, after */ + 'use strict'; -var preq = require('preq'); -var assert = require('../../utils/assert.js'); -var server = require('../../utils/server.js'); -var URI = require('swagger-router').URI; -var yaml = require('js-yaml'); -var fs = require('fs'); +const preq = require('preq'); +const assert = require('../../utils/assert.js'); +const server = require('../../utils/server.js'); +const URI = require('swagger-router').URI; +const yaml = require('js-yaml'); +const fs = require('fs'); +if (!server.stopHookAdded) { + server.stopHookAdded = true; + after(() => server.stop()); +} function staticSpecLoad() { - var spec; - var myService = server.config.conf.services[server.config.conf.services.length - 1].conf; - var specPath = __dirname + '/../../../' + (myService.spec ? myService.spec : 'spec.yaml'); + let spec; + const myService = server.config.conf.services[server.config.conf.services.length - 1].conf; + const specPath = `${__dirname}/../../../${myService.spec ? myService.spec : 'spec.yaml'}`; try { spec = yaml.safeLoad(fs.readFileSync(specPath)); - } catch(e) { + } catch (e) { // this error will be detected later, so ignore it - spec = {paths: {}, 'x-default-params': {}}; + spec = { paths: {}, 'x-default-params': {} }; } return spec; @@ -29,30 +35,32 @@ function validateExamples(pathStr, defParams, mSpec) { - var uri = new URI(pathStr, {}, true); + const uri = new URI(pathStr, {}, true); - if(!mSpec) { + if (!mSpec) { try { uri.expand(defParams); return true; - } catch(e) { - throw new Error('Missing parameter for route ' + pathStr + ' : ' + e.message); + } catch (e) { + throw new Error(`Missing parameter for route ${pathStr} : ${e.message}`); } } - if(!Array.isArray(mSpec)) { - throw new Error('Route ' + pathStr + ' : x-amples must be an array!'); + if (!Array.isArray(mSpec)) { + throw new Error(`Route ${pathStr} : x-amples must be an array!`); } - mSpec.forEach(function(ex, idx) { - if(!ex.title) { - throw new Error('Route ' + pathStr + ', example ' + idx + ': title missing!'); + mSpec.forEach((ex, idx) => { + if (!ex.title) { + throw new Error(`Route ${pathStr}, example ${idx}: title missing!`); } ex.request = ex.request || {}; try { uri.expand(Object.assign({}, defParams, ex.request.params || {})); - } catch(e) { - throw new Error('Route ' + pathStr + ', example ' + idx + ' (' + ex.title + '): missing parameter: ' + e.message); + } catch (e) { + throw new Error( + `Route ${pathStr}, example ${idx} (${ex.title}): missing parameter: ${e.message}` + ); } }); @@ -64,10 +72,10 @@ function constructTestCase(title, path, method, request, response) { return { - title: title, + title, request: { uri: server.config.uri + (path[0] === '/' ? path.substr(1) : path), - method: method, + method, headers: request.headers || {}, query: request.query, body: request.body, @@ -85,31 +93,30 @@ function constructTests(paths, defParams) { - var ret = []; + const ret = []; - Object.keys(paths).forEach(function(pathStr) { - Object.keys(paths[pathStr]).forEach(function(method) { - var p = paths[pathStr][method]; - var uri; - if(p.hasOwnProperty('x-monitor') && !p['x-monitor']) { + Object.keys(paths).forEach((pathStr) => { + Object.keys(paths[pathStr]).forEach((method) => { + const p = paths[pathStr][method]; + if ({}.hasOwnProperty.call(p, 'x-monitor') && !p['x-monitor']) { return; } - uri = new URI(pathStr, {}, true); - if(!p['x-amples']) { + const uri = new URI(pathStr, {}, true); + if (!p['x-amples']) { ret.push(constructTestCase( pathStr, - uri.toString({params: defParams}), + uri.toString({ params: defParams }), method, {}, {} )); return; } - p['x-amples'].forEach(function(ex) { + p['x-amples'].forEach((ex) => { ex.request = ex.request || {}; ret.push(constructTestCase( ex.title, - uri.toString({params: Object.assign({}, defParams, ex.request.params || {})}), + uri.toString({ params: Object.assign({}, defParams, ex.request.params || {}) }), method, ex.request, ex.response || {} @@ -125,51 +132,52 @@ function cmp(result, expected, errMsg) { - if(expected === null || expected === undefined) { + if (expected === null || expected === undefined) { // nothing to expect, so we can return return true; } - if(result === null || result === undefined) { + if (result === null || result === undefined) { result = ''; } - if(expected.constructor === Object) { - Object.keys(expected).forEach(function(key) { - var val = expected[key]; - assert.deepEqual(result.hasOwnProperty(key), true, 'Body field ' + key + ' not found in response!'); - cmp(result[key], val, key + ' body field mismatch!'); + if (expected.constructor === Object) { + Object.keys(expected).forEach((key) => { + const val = expected[key]; + assert.deepEqual({}.hasOwnProperty.call(result, key), true, + `Body field ${key} not found in response!`); + cmp(result[key], val, `${key} body field mismatch!`); }); return true; - } else if(expected.constructor === Array) { - if(result.constructor !== Array) { + } else if (expected.constructor === Array) { + if (result.constructor !== Array) { assert.deepEqual(result, expected, errMsg); return true; } // only one item in expected - compare them all - if(expected.length === 1 && result.length > 1) { - result.forEach(function(item) { + if (expected.length === 1 && result.length > 1) { + result.forEach((item) => { cmp(item, expected[0], errMsg); }); return true; } // more than one item expected, check them one by one - if(expected.length !== result.length) { + if (expected.length !== result.length) { assert.deepEqual(result, expected, errMsg); return true; } - expected.forEach(function(item, idx) { + expected.forEach((item, idx) => { cmp(result[idx], item, errMsg); }); return true; } - if(expected.length > 1 && expected[0] === '/' && expected[expected.length - 1] === '/') { - if((new RegExp(expected.slice(1, -1))).test(result)) { + if (expected.length > 1 && expected[0] === '/' && expected[expected.length - 1] === '/') { + if ((new RegExp(expected.slice(1, -1))).test(result)) { return true; } - } else if(expected.length === 0 && result.length === 0) { + } else if (expected.length === 0 && result.length === 0) { return true; - } else if(result === expected || result.startsWith(expected)) { + } else if (result === expected || result.startsWith(expected)) { return true; } @@ -181,32 +189,35 @@ function validateTestResponse(testCase, res) { - var expRes = testCase.response; + const expRes = testCase.response; // check the status assert.status(res, expRes.status); // check the headers - Object.keys(expRes.headers).forEach(function(key) { - var val = expRes.headers[key]; - assert.deepEqual(res.headers.hasOwnProperty(key), true, 'Header ' + key + ' not found in response!'); - cmp(res.headers[key], val, key + ' header mismatch!'); + Object.keys(expRes.headers).forEach((key) => { + const val = expRes.headers[key]; + assert.deepEqual({}.hasOwnProperty.call(res.headers, key), true, + `Header ${key} not found in response!`); + cmp(res.headers[key], val, `${key} header mismatch!`); }); // check the body - if(!expRes.body) { + if (!expRes.body) { return true; } res.body = res.body || ''; - if(Buffer.isBuffer(res.body)) { res.body = res.body.toString(); } - if(expRes.body.constructor !== res.body.constructor) { - if(expRes.body.constructor === String) { + if (Buffer.isBuffer(res.body)) { res.body = res.body.toString(); } + if (expRes.body.constructor !== res.body.constructor) { + if (expRes.body.constructor === String) { res.body = JSON.stringify(res.body); } else { res.body = JSON.parse(res.body); } } // check that the body type is the same - if(expRes.body.constructor !== res.body.constructor) { - throw new Error('Expected a body of type ' + expRes.body.constructor + ' but gotten ' + res.body.constructor); + if (expRes.body.constructor !== res.body.constructor) { + throw new Error( + `Expected body type ${expRes.body.constructor} but got ${res.body.constructor}` + ); } // compare the bodies @@ -220,19 +231,19 @@ describe('Swagger spec', function() { // the variable holding the spec - var spec = staticSpecLoad(); + let spec = staticSpecLoad(); // default params, if given - var defParams = spec['x-default-params'] || {}; + let defParams = spec['x-default-params'] || {}; - this.timeout(20000); + this.timeout(20000); // eslint-disable-line no-invalid-this - before(function () { + before(() => { return server.start(); }); - it('get the spec', function() { - return preq.get(server.config.uri + '?spec') - .then(function(res) { + it('get the spec', () => { + return preq.get(`${server.config.uri}?spec`) + .then((res) => { assert.status(200); assert.contentType(res, 'application/json'); assert.notDeepEqual(res.body, undefined, 'No body received!'); @@ -240,25 +251,24 @@ }); }); - it('spec validation', function() { - if(spec['x-default-params']) { + it('spec validation', () => { + if (spec['x-default-params']) { defParams = spec['x-default-params']; } // check the high-level attributes - ['info', 'swagger', 'paths'].forEach(function(prop) { - assert.deepEqual(!!spec[prop], true, 'No ' + prop + ' field present!'); + ['info', 'swagger', 'paths'].forEach((prop) => { + assert.deepEqual(!!spec[prop], true, `No ${prop} field present!`); }); // no paths - no love assert.deepEqual(!!Object.keys(spec.paths), true, 'No paths given in the spec!'); // now check each path - Object.keys(spec.paths).forEach(function(pathStr) { - var path; + Object.keys(spec.paths).forEach((pathStr) => { assert.deepEqual(!!pathStr, true, 'A path cannot have a length of zero!'); - path = spec.paths[pathStr]; - assert.deepEqual(!!Object.keys(path), true, 'No methods defined for path: ' + pathStr); - Object.keys(path).forEach(function(method) { - var mSpec = path[method]; - if(mSpec.hasOwnProperty('x-monitor') && !mSpec['x-monitor']) { + const path = spec.paths[pathStr]; + assert.deepEqual(!!Object.keys(path), true, `No methods defined for path: ${pathStr}`); + Object.keys(path).forEach((method) => { + const mSpec = path[method]; + if ({}.hasOwnProperty.call(mSpec, 'x-monitor') && !mSpec['x-monitor']) { return; } validateExamples(pathStr, defParams, mSpec['x-amples']); @@ -266,20 +276,19 @@ }); }); - describe('routes', function() { + describe('routes', () => { - constructTests(spec.paths, defParams).forEach(function(testCase) { - it(testCase.title, function() { + constructTests(spec.paths, defParams).forEach((testCase) => { + it(testCase.title, () => { return preq(testCase.request) - .then(function(res) { + .then((res) => { validateTestResponse(testCase, res); - }, function(err) { + }, (err) => { validateTestResponse(testCase, err); }); }); }); }); - }); diff --git a/test/features/info/info.js b/test/features/info/info.js index a866dfb..02767dd 100644 --- a/test/features/info/info.js +++ b/test/features/info/info.js @@ -1,58 +1,64 @@ +/* global describe, it, before, after */ + 'use strict'; -var preq = require('preq'); -var assert = require('../../utils/assert.js'); -var server = require('../../utils/server.js'); +const preq = require('preq'); +const assert = require('../../utils/assert.js'); +const server = require('../../utils/server.js'); +if (!server.stopHookAdded) { + server.stopHookAdded = true; + after(() => server.stop()); +} describe('service information', function() { - this.timeout(20000); + this.timeout(20000); // eslint-disable-line no-invalid-this - before(function () { return server.start(); }); + before(() => { return server.start(); }); // common URI prefix for info tests - var infoUri = server.config.uri + '_info/'; + const infoUri = `${server.config.uri}_info/`; // common function used for generating requests // and checking their return values function checkRet(fieldName) { return preq.get({ uri: infoUri + fieldName - }).then(function(res) { + }).then((res) => { // check the returned Content-Type header assert.contentType(res, 'application/json'); // the status as well assert.status(res, 200); // finally, check the body has the specified field assert.notDeepEqual(res.body, undefined, 'No body returned!'); - assert.notDeepEqual(res.body[fieldName], undefined, 'No ' + fieldName + ' field returned!'); + assert.notDeepEqual(res.body[fieldName], undefined, `No ${fieldName} field returned!`); }); } - it('should get the service name', function() { + it('should get the service name', () => { return checkRet('name'); }); - it('should get the service version', function() { + it('should get the service version', () => { return checkRet('version'); }); - it('should redirect to the service home page', function() { + it('should redirect to the service home page', () => { return preq.get({ - uri: infoUri + 'home', + uri: `${infoUri}home`, followRedirect: false - }).then(function(res) { + }).then((res) => { // check the status assert.status(res, 301); }); }); - it('should get the service info', function() { + it('should get the service info', () => { return preq.get({ uri: infoUri - }).then(function(res) { + }).then((res) => { // check the status assert.status(res, 200); // check the returned Content-Type header @@ -65,6 +71,5 @@ assert.notDeepEqual(res.body.home, undefined, 'No home field returned!'); }); }); - }); diff --git a/test/index.js b/test/index.js index 535299f..b145c8a 100644 --- a/test/index.js +++ b/test/index.js @@ -3,4 +3,9 @@ // Run jshint as part of normal testing require('mocha-jshint')(); - +require('mocha-eslint')([ + 'lib', + 'routes' +], { + timeout: 10000 +}); diff --git a/test/utils/assert.js b/test/utils/assert.js index ed030ea..dfbcdef 100644 --- a/test/utils/assert.js +++ b/test/utils/assert.js @@ -1,7 +1,26 @@ +/* eslint-disable no-console */ + 'use strict'; -var assert = require('assert'); +const assert = require('assert'); + + +function deepEqual(result, expected, message) { + + try { + if (typeof expected === 'string') { + assert.ok(result === expected || (new RegExp(expected).test(result))); + } else { + assert.deepEqual(result, expected, message); + } + } catch (e) { + console.log(`Expected:\n${JSON.stringify(expected, null, 2)}`); + console.log(`Result:\n${JSON.stringify(result, null, 2)}`); + throw e; + } + +} /** @@ -10,7 +29,7 @@ function status(res, expected) { deepEqual(res.status, expected, - 'Expected status to be ' + expected + ', but was ' + res.status); + `Expected status to be ${expected}, but was ${res.status}`); } @@ -20,9 +39,9 @@ */ function contentType(res, expected) { - var actual = res.headers['content-type']; + const actual = res.headers['content-type']; deepEqual(actual, expected, - 'Expected content-type to be ' + expected + ', but was ' + actual); + `Expected content-type to be ${expected}, but was ${actual}`); } @@ -43,30 +62,13 @@ } -function deepEqual(result, expected, message) { - - try { - if (typeof expected === 'string') { - assert.ok(result === expected || (new RegExp(expected).test(result))); - } else { - assert.deepEqual(result, expected, message); - } - } catch (e) { - console.log('Expected:\n' + JSON.stringify(expected, null, 2)); - console.log('Result:\n' + JSON.stringify(result, null, 2)); - throw e; - } - -} - - function notDeepEqual(result, expected, message) { try { assert.notDeepEqual(result, expected, message); } catch (e) { - console.log('Not expected:\n' + JSON.stringify(expected, null, 2)); - console.log('Result:\n' + JSON.stringify(result, null, 2)); + console.log(`Not expected:\n${JSON.stringify(expected, null, 2)}`); + console.log(`Result:\n${JSON.stringify(result, null, 2)}`); throw e; } @@ -75,7 +77,7 @@ function fails(promise, onRejected) { - var failed = false; + let failed = false; function trackFailure(e) { failed = true; diff --git a/test/utils/logStream.js b/test/utils/logStream.js index f8e292d..3872fb7 100644 --- a/test/utils/logStream.js +++ b/test/utils/logStream.js @@ -1,68 +1,72 @@ +/* eslint-disable no-console */ + 'use strict'; -var bunyan = require('bunyan'); +const bunyan = require('bunyan'); function logStream(logStdout) { - var log = []; - var parrot = bunyan.createLogger({ - name: 'test-logger', - level: 'warn' - }); + const log = []; + const parrot = bunyan.createLogger({ + name: 'test-logger', + level: 'warn' + }); - function write(chunk, encoding, callback) { - try { - var entry = JSON.parse(chunk); - var levelMatch = /^(\w+)/.exec(entry.levelPath); - if (logStdout && levelMatch) { - var level = levelMatch[1]; - if (parrot[level]) { - parrot[level](entry); + function write(chunk, encoding, callback) { + try { + const entry = JSON.parse(chunk); + const levelMatch = /^(\w+)/.exec(entry.levelPath); + if (logStdout && levelMatch) { + const level = levelMatch[1]; + if (parrot[level]) { + parrot[level](entry); + } } + } catch (e) { + console.error('something went wrong trying to parrot a log entry', e, chunk); } - } catch (e) { - console.error('something went wrong trying to parrot a log entry', e, chunk); + + log.push(chunk); } - log.push(chunk); - } - - // to implement the stream writer interface - function end(chunk, encoding, callback) { - } - - function get() { - return log; - } - - function slice() { - - var begin = log.length; - var end = null; - - function halt() { - if (end === null) { - end = log.length; - } + // to implement the stream writer interface + function end(chunk, encoding, callback) { } function get() { - return log.slice(begin, end); + return log; + } + + function slice() { + + const begin = log.length; + let end = null; + + function halt() { + if (end === null) { + end = log.length; + } + } + + function get() { + return log.slice(begin, end); + } + + /* Disable eslint object-shorthand until Node 4 support is dropped */ + /* eslint-disable object-shorthand */ + return { + halt: halt, + get: get + }; + } return { - halt: halt, - get: get + write: write, + end: end, + slice: slice, + get: get }; - - } - - return { - write: write, - end: end, - slice: slice, - get: get - }; } module.exports = logStream; diff --git a/test/utils/server.js b/test/utils/server.js index ba0d718..f9d0127 100644 --- a/test/utils/server.js +++ b/test/utils/server.js @@ -1,28 +1,24 @@ 'use strict'; -// mocha defines to avoid JSHint breakage -/* global describe, it, before, beforeEach, after, afterEach */ - - -var BBPromise = require('bluebird'); -var ServiceRunner = require('service-runner'); -var logStream = require('./logStream'); -var fs = require('fs'); -var assert = require('./assert'); -var yaml = require('js-yaml'); -var extend = require('extend'); +const BBPromise = require('bluebird'); +const ServiceRunner = require('service-runner'); +const logStream = require('./logStream'); +const fs = require('fs'); +const assert = require('./assert'); +const yaml = require('js-yaml'); +const extend = require('extend'); // set up the configuration -var config = { - conf: yaml.safeLoad(fs.readFileSync(__dirname + '/../../config.yaml')) +let config = { + conf: yaml.safeLoad(fs.readFileSync(`${__dirname}/../../config.yaml`)) }; // build the API endpoint URI by supposing the actual service // is the last one in the 'services' list in the config file -var myServiceIdx = config.conf.services.length - 1; -var myService = config.conf.services[myServiceIdx]; -config.uri = 'http://localhost:' + myService.conf.port + '/'; +const myServiceIdx = config.conf.services.length - 1; +const myService = config.conf.services[myServiceIdx]; +config.uri = `http://localhost:${myService.conf.port}/`; config.service = myService; // no forking, run just one process when testing config.conf.num_workers = 0; @@ -33,11 +29,11 @@ stream: logStream() }; // make a deep copy of it for later reference -var origConfig = extend(true, {}, config); +const origConfig = extend(true, {}, config); -var stop = function() { return BBPromise.resolve(); }; -var options = null; -var runner = new ServiceRunner(); +module.exports.stop = () => { return BBPromise.resolve(); }; +let options = null; +const runner = new ServiceRunner(); function start(_options) { @@ -45,18 +41,23 @@ _options = _options || {}; if (!assert.isDeepEqual(options, _options)) { - console.log('server options changed; restarting'); - return stop().then(function() { + console.log('starting test server'); // eslint-disable-line no-console + return module.exports.stop().then(() => { options = _options; // set up the config config = extend(true, {}, origConfig); extend(true, config.conf.services[myServiceIdx].conf, options); return runner.start(config.conf) - .then(function() { - stop = function () { - console.log('stopping test server'); - return runner.stop().then(function() { - stop = function() { return BBPromise.resolve(); }; + .then((serviceReturns) => { + module.exports.stop = () => { + console.log('stopping test server'); // eslint-disable-line no-console + serviceReturns.forEach(servers => + servers.forEach(server => + server.shutdown())); + return runner.stop().then(() => { + module.exports.stop = () => { + return BBPromise.resolve(); + }; }); }; return true; @@ -65,7 +66,6 @@ } else { return BBPromise.resolve(); } - } module.exports.config = config; -- To view, visit https://gerrit.wikimedia.org/r/398031 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I21bb4113a19810e04e099fd7edcade44450ba6af Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/services/graphoid Gerrit-Branch: master Gerrit-Owner: Mobrovac <mobro...@wikimedia.org> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits