Mobrovac has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/391985 )
Change subject: Update the service template to 0.5.3
......................................................................
Update the service template to 0.5.3
This updates to the latest version of
ServiceTemplateNode (0.5.3)
A forced merged with --allow-unrelated-histories
was necessary.
eslint rules are temporarily disabled in various files
not specific to the ServiceTemplate
and will be fixed in a follow up (076e54f)
Bug: T151395
Change-Id: Ie0d99fc300aa3978056cd96666124383cd76cb11
---
A .eslintrc.yml
M .gitignore
M .travis.yml
M app.js
M config.prod.yaml
M lib/EventProcessor.js
M lib/EventSource.js
M lib/api-util.js
M lib/score-pages.js
A lib/swagger-ui.js
M lib/util.js
M package.json
M routes/info.js
M routes/root.js
M routes/trending-v1.js
M server.js
M test/features/app/app.js
M test/features/app/spec.js
A test/features/info/info.js
M test/features/v1/trending.js
M test/index.js
M test/utils/logStream.js
M test/utils/server.js
23 files changed, 582 insertions(+), 398 deletions(-)
Approvals:
Mobrovac: Verified; Looks good to me, approved
jenkins-bot: Verified
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/.gitignore b/.gitignore
index b09b5ed..850a956 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,5 +1,6 @@
Dockerfile
.idea/
coverage
+config.yaml
node_modules
npm-debug.log
diff --git a/.travis.yml b/.travis.yml
index f33adc7..b6a9e5d 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -3,7 +3,7 @@
sudo: false
node_js:
- - "0.10"
- - "0.12"
- "4"
- - "5"
+ - "6"
+ - "8"
+ - "node"
diff --git a/app.js b/app.js
index 7f5cf81..ed8ae1d 100644
--- a/app.js
+++ b/app.js
@@ -1,20 +1,18 @@
'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');
-var EventSource = require('./lib/EventSource');
-var EventProcessor = require('./lib/EventProcessor');
+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');
+const EventSource = require('./lib/EventSource');
+const EventProcessor = require('./lib/EventProcessor');
/**
@@ -25,7 +23,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
@@ -43,8 +41,8 @@
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'";
+ // 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
@@ -64,26 +62,26 @@
// set up header whitelisting for logging
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';
+ app.conf.spec = `${__dirname}/spec.yaml`;
}
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);
+ app.logger.log('warn/spec', `Could not load the spec: ${e}`);
app.conf.spec = {};
}
}
@@ -103,7 +101,7 @@
}
// set the CORS and CSP headers
- app.all('*', function(req, res, next) {
+ 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');
@@ -143,45 +141,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) {
// 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)) {
return undefined;
}
// import the route file
- var route = require(__dirname + '/routes/' + fname);
+ const route = require(`${__dirname}/routes/${fname}`);
return route(app);
- }).then(function(route) {
+ }).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;
+ route.path = `/${route.path}`;
}
if (route.path[route.path.length - 1] !== '/') {
- route.path = route.path + '/';
+ route.path = `${route.path}/`;
}
if (!route.skip_domain) {
- route.path = '/:domain/v' + route.api_version + route.path;
+ 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
@@ -218,40 +217,48 @@
editStream.on('error', (e) => {
app.logger.log('error/edit_stream', 'Error in edit stream: ' + e);
});
- process.on('exit', () => {
- clearInterval(app._metricsReporter);
- editStream.close();
- });
+ app.editStream = editStream;
return app;
}
/**
* 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", function(socket) {
+ server.on("connection", (socket) => {
socket.setNoDelay(true);
});
+
+ // Clean up on exit
+ const server_close = () => {
+ try {
+ clearInterval(app._metricsReporter);
+ app.editStream.close();
+ } catch (e) { /* Ignore any errors */ }
+ };
+ server.on('close', server_close);
+ process.on('exit', server_close);
return server;
});
@@ -270,9 +277,9 @@
return initApp(options)
.then(createEditStream)
.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/config.prod.yaml b/config.prod.yaml
index b58c8cf..dfbe263 100644
--- a/config.prod.yaml
+++ b/config.prod.yaml
@@ -89,3 +89,4 @@
min_editors: 3
min_edits: 6
broker_list: localhost:9092
+
diff --git a/lib/EventProcessor.js b/lib/EventProcessor.js
index 29f483d..9bd734a 100644
--- a/lib/EventProcessor.js
+++ b/lib/EventProcessor.js
@@ -1,4 +1,6 @@
"use strict";
+
+/* eslint-disable */
const EventEmitter = require('events').EventEmitter;
/**
diff --git a/lib/EventSource.js b/lib/EventSource.js
index 2c82247..34f9f33 100644
--- a/lib/EventSource.js
+++ b/lib/EventSource.js
@@ -1,5 +1,6 @@
"use strict";
+/* eslint-disable */
const kafka = require('node-rdkafka');
const EventEmitter = require('events').EventEmitter;
const os = require('os');
diff --git a/lib/api-util.js b/lib/api-util.js
index 67f1710..cef871e 100644
--- a/lib/api-util.js
+++ b/lib/api-util.js
@@ -1,35 +1,32 @@
'use strict';
-
-var preq = require('preq');
-var sUtil = require('./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) {
+ 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({
@@ -47,16 +44,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 +60,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,8 +77,7 @@
/**
* 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) {
@@ -114,8 +109,8 @@
module.exports = {
- mwApiGet: mwApiGet,
- restApiGet: restApiGet,
- setupApiTemplates: setupApiTemplates
+ mwApiGet,
+ restApiGet,
+ setupApiTemplates
};
diff --git a/lib/score-pages.js b/lib/score-pages.js
index 0c0e842..a5891ee 100644
--- a/lib/score-pages.js
+++ b/lib/score-pages.js
@@ -1,5 +1,6 @@
"use strict";
+/* eslint-disable */
var scorer = require('wikipedia-edits-scorer');
/**
diff --git a/lib/swagger-ui.js b/lib/swagger-ui.js
new file mode 100644
index 0000000..9e39ff5
--- /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 16fc4a1..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,
@@ -63,7 +56,7 @@
};
if (req.headers && whitelistRE) {
- Object.keys(req.headers).forEach(function(hdr) {
+ 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;
+ .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) {
errObj = err;
} else if (err instanceof Error) {
// is this an HTTPError defined elsewhere? (preq)
if (err.constructor.name === 'HTTPError') {
- var o = { status: err.status };
+ const o = { status: err.status };
if (err.body && err.body.constructor === Object) {
- Object.keys(err.body).forEach(function(key) {
+ Object.keys(err.body).forEach((key) => {
o[key] = err.body[key];
});
} else {
@@ -211,18 +197,17 @@
// 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];
- });
+ 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 = 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/package.json b/package.json
index 40b3d40..9d8d083 100644
--- a/package.json
+++ b/package.json
@@ -1,14 +1,15 @@
{
"name": "trending-edits",
- "version": "0.1.0",
+ "version": "0.1.1",
"description": "A trending REST API service based on edits",
"homepage":
"https://www.mediawiki.org/wiki/Reading/Web/Projects/Edit_based_trending_service";,
"main": "./app.js",
"scripts": {
"start": "service-runner",
- "test": "mocha && nsp check",
+ "test": "PREQ_CONNECT_TIMEOUT=15 mocha && nsp check",
"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": {
@@ -20,31 +21,42 @@
"bugs": {
"url":
"https://phabricator.wikimedia.org/tag/reading_web_trending_service/";
},
+ "keywords": [
+ "Trending",
+ "API",
+ "edits",
+ "MediaWiki"
+ ],
"dependencies": {
"bluebird": "^3.5.1",
"body-parser": "^1.18.2",
"bunyan": "^1.8.12",
"cassandra-uuid": "^0.0.2",
"compression": "^1.7.1",
- "core-js": "^2.5.1",
"domino": "^1.0.30",
- "express": "^4.14.0",
+ "express": "^4.16.2",
"js-yaml": "^3.10.0",
- "preq": "^0.4.10",
- "service-runner": "^2.2.5",
- "swagger-router": "^0.4.6",
+ "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",
"node-rdkafka": "^1.0.6",
"wikipedia-edits-scorer": "^1.5.7"
},
"devDependencies": {
"extend": "^3.0.1",
- "istanbul": "^0.4.4",
- "jscs": "^3.0.7",
- "mocha": "^2.5.3",
- "mocha-jscs": "^5.0.1",
+ "istanbul": "^0.4.5",
+ "mocha": "^4.0.1",
"mocha-jshint": "^2.3.1",
"mocha-lcov-reporter": "^1.3.0",
- "nsp": "^2.6.1"
+ "nsp": "^2.8.1",
+ "mocha-eslint": "^3.0.1",
+ "eslint": "^3.12.0",
+ "eslint-config-node-services": "^2.0.2",
+ "eslint-config-wikimedia": "^0.4.0",
+ "eslint-plugin-json": "^1.2.0",
+ "eslint-plugin-jsdoc": "^3.0.0"
},
"deploy": {
"node": "6.11.1",
diff --git a/routes/info.js b/routes/info.js
index 79b82e3..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,28 +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;
+ const home = app.info.homepage;
if (home && /^http/.test(home)) {
// we have a home page URI defined, so send it
res.redirect(301, home);
} 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 31d5c40..83ee521 100644
--- a/routes/root.js
+++ b/routes/root.js
@@ -1,29 +1,30 @@
'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': '*',
- Disallow: '/'
+ 'Disallow': '/'
}).end();
});
@@ -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/routes/trending-v1.js b/routes/trending-v1.js
index 050b4a5..aec72a6 100644
--- a/routes/trending-v1.js
+++ b/routes/trending-v1.js
@@ -1,6 +1,6 @@
'use strict';
-
+/* eslint-disable */
var BBPromise = require('bluebird');
var preq = require('preq');
var domino = require('domino');
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..3cef837 100644
--- a/test/features/app/app.js
+++ b/test/features/app/app.js
@@ -1,33 +1,37 @@
'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 +39,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 +56,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..013be6b 100644
--- a/test/features/app/spec.js
+++ b/test/features/app/spec.js
@@ -1,25 +1,28 @@
'use strict';
+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');
-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');
-
+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 +32,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 +69,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 +90,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 +129,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 +186,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 +228,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 +248,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 +273,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
new file mode 100644
index 0000000..65b8551
--- /dev/null
+++ b/test/features/info/info.js
@@ -0,0 +1,73 @@
+'use strict';
+
+
+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);
+
+ before(() => { return server.start(); });
+
+ // common URI prefix for info tests
+ 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((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!`);
+ });
+ }
+
+ it('should get the service name', () => {
+ return checkRet('name');
+ });
+
+ it('should get the service version', () => {
+ return checkRet('version');
+ });
+
+ it('should redirect to the service home page', () => {
+ return preq.get({
+ uri: `${infoUri}home`,
+ followRedirect: false
+ }).then((res) => {
+ // check the status
+ assert.status(res, 301);
+ });
+ });
+
+ it('should get the service info', () => {
+ return preq.get({
+ uri: infoUri
+ }).then((res) => {
+ // check the status
+ assert.status(res, 200);
+ // check the returned Content-Type header
+ assert.contentType(res, 'application/json');
+ // inspect the body
+ assert.notDeepEqual(res.body, undefined, 'No body returned!');
+ assert.notDeepEqual(res.body.name, undefined, 'No name field
returned!');
+ assert.notDeepEqual(res.body.version, undefined, 'No version field
returned!');
+ assert.notDeepEqual(res.body.description, undefined, 'No
description field returned!');
+ assert.notDeepEqual(res.body.home, undefined, 'No home field
returned!');
+ });
+ });
+});
+
diff --git a/test/features/v1/trending.js b/test/features/v1/trending.js
index 3f1a4a2..0ec81ae 100644
--- a/test/features/v1/trending.js
+++ b/test/features/v1/trending.js
@@ -6,6 +6,11 @@
const server = require('../../utils/server.js');
const fixtures = require('../../utils/fixtures');
+if (!server.stopHookAdded) {
+ server.stopHookAdded = true;
+ after(() => server.stop());
+}
+
describe('trending', function() {
this.timeout(20000);
diff --git a/test/index.js b/test/index.js
index c360f7d..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-jscs")();
+require('mocha-eslint')([
+ 'lib',
+ 'routes'
+], {
+ timeout: 10000
+});
diff --git a/test/utils/logStream.js b/test/utils/logStream.js
index 842df59..f8e292d 100644
--- a/test/utils/logStream.js
+++ b/test/utils/logStream.js
@@ -4,65 +4,65 @@
function logStream(logStdout) {
- var log = [];
- var parrot = bunyan.createLogger({
- name: 'test-logger',
- level: 'warn'
- });
+ var log = [];
+ var 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 {
+ var entry = JSON.parse(chunk);
+ var levelMatch = /^(\w+)/.exec(entry.levelPath);
+ if (logStdout && levelMatch) {
+ var level = levelMatch[1];
+ if (parrot[level]) {
+ parrot[level](entry);
}
- } catch (e) {
- console.error('something went wrong trying to parrot a log entry',
e, chunk);
}
-
- log.push(chunk);
+ } catch (e) {
+ console.error('something went wrong trying to parrot a log entry', e,
chunk);
}
- // to implement the stream writer interface
- function end(chunk, encoding, callback) {
+ 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;
+ }
}
function get() {
- return log;
- }
-
- function slice() {
-
- var begin = log.length;
- var end = null;
-
- function halt() {
- if (end === null) {
- end = log.length;
- }
- }
-
- function get() {
- return log.slice(begin, end);
- }
-
- return {
- halt: halt,
- get: get
- };
-
+ return log.slice(begin, end);
}
return {
- write: write,
- end: end,
- slice: slice,
- get: get
+ halt: halt,
+ 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..8a67d3f 100644
--- a/test/utils/server.js
+++ b/test/utils/server.js
@@ -5,24 +5,24 @@
/* 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 +33,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 +45,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 +70,6 @@
} else {
return BBPromise.resolve();
}
-
}
module.exports.config = config;
--
To view, visit https://gerrit.wikimedia.org/r/391985
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie0d99fc300aa3978056cd96666124383cd76cb11
Gerrit-PatchSet: 4
Gerrit-Project: mediawiki/services/trending-edits
Gerrit-Branch: master
Gerrit-Owner: Jdlrobson <jrob...@wikimedia.org>
Gerrit-Reviewer: BearND <bsitzm...@wikimedia.org>
Gerrit-Reviewer: Jdlrobson <jrob...@wikimedia.org>
Gerrit-Reviewer: Mholloway <mhollo...@wikimedia.org>
Gerrit-Reviewer: Mobrovac <mobro...@wikimedia.org>
Gerrit-Reviewer: Ppchelko <ppche...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
