jenkins-bot has submitted this change and it was merged. ( https://gerrit.wikimedia.org/r/390878 )
Change subject: Update to service-template-node v0.5.3 ...................................................................... Update to service-template-node v0.5.3 Bug: T175758 Change-Id: Id6a2fe9c8656ce9947e2b9936275a40db704e402 --- M .eslintrc.yml M .travis.yml M app.js M lib/api-util.js A lib/swagger-ui.js M lib/util.js M package.json M routes/root.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/server.js 13 files changed, 161 insertions(+), 48 deletions(-) Approvals: Mobrovac: Looks good to me, approved jenkins-bot: Verified diff --git a/.eslintrc.yml b/.eslintrc.yml index bf9f475..312fdfd 100644 --- a/.eslintrc.yml +++ b/.eslintrc.yml @@ -1 +1 @@ -extends: node-services \ No newline at end of file +extends: 'eslint-config-node-services' diff --git a/.travis.yml b/.travis.yml index c417c7f..b6a9e5d 100644 --- a/.travis.yml +++ b/.travis.yml @@ -5,4 +5,5 @@ node_js: - "4" - "6" + - "8" - "node" diff --git a/app.js b/app.js index 413ff72..8c9961d 100644 --- a/app.js +++ b/app.js @@ -13,6 +13,7 @@ const apiUtil = require('./lib/api-util'); const packageInfo = require('./package.json'); const yaml = require('js-yaml'); +const addShutdown = require('http-shutdown'); /** @@ -158,7 +159,7 @@ } // check that the route exports the object we need if (route.constructor !== Object || !route.path || !route.router - || !(route.api_version || route.skip_domain)) { + || !(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 @@ -203,6 +204,7 @@ app.conf.interface, resolve ); + server = addShutdown(server); }).then(() => { app.logger.log('info', `Worker ${process.pid} listening on ${app.conf.interface || '*'}:${app.conf.port}`); diff --git a/lib/api-util.js b/lib/api-util.js index 9fd9959..d6059d7 100644 --- a/lib/api-util.js +++ b/lib/api-util.js @@ -4,7 +4,6 @@ const preq = require('preq'); const sUtil = require('./util'); const Template = require('swagger-router').Template; - const HTTPError = sUtil.HTTPError; 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 99f4e72..8274748 100644 --- a/lib/util.js +++ b/lib/util.js @@ -37,7 +37,7 @@ } /** - * Generates an object suitable for logging out of a request object. + * 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 diff --git a/package.json b/package.json index f1a64ef..56d09f6 100644 --- a/package.json +++ b/package.json @@ -10,6 +10,7 @@ "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": { @@ -40,39 +41,42 @@ }, "homepage": "https://www.mediawiki.org/wiki/RESTBase_services_for_apps", "dependencies": { - "bluebird": "^3.4.6", - "body-parser": "^1.15.2", - "bunyan": "^1.8.5", + "bluebird": "^3.5.1", + "body-parser": "^1.18.2", + "bunyan": "^1.8.12", "cassandra-uuid": "^0.0.2", - "compression": "^1.6.2", + "compression": "^1.7.1", "core-js": "^2.4.1", - "domino": "^1.0.27", + "domino": "^1.0.30", "escape-string-regexp": "^1.0.5", - "express": "^4.16.0", - "js-yaml": "^3.7.0", + "express": "^4.16.2", + "js-yaml": "^3.10.0", "mediawiki-title": "^0.6.3", "parsoid-dom-utils": "^0.1.3", - "preq": "^0.5.1", - "service-runner": "^2.2.5", - "swagger-router": "^0.5.5", + "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" }, "devDependencies": { "ajv": "^4.7.7", "csv-parse": "^1.1.7", - "eslint": "^3.11.1", - "eslint-config-node-services": "^2.2.2", - "eslint-config-wikimedia": "^0.4.0", - "eslint-plugin-jsdoc": "^3.0.0", - "eslint-plugin-json": "^1.2.0", - "extend": "^3.0.0", + "extend": "^3.0.1", "istanbul": "^0.4.5", "js-beautify": "^1.6.8", "mkdirp": "^0.5.1", - "mocha": "^3.1.2", + "mocha": "^4.0.1", "mocha-jshint": "^2.3.1", - "mocha-lcov-reporter": "^1.2.0", - "nsp": "^2.6.2", + "mocha-lcov-reporter": "^1.3.0", + "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", "rss-parser": "^2.5.2", "sepia": "^2.0.1" }, diff --git a/routes/root.js b/routes/root.js index cf0ffda..83ee521 100644 --- a/routes/root.js +++ b/routes/root.js @@ -2,6 +2,7 @@ const sUtil = require('../lib/util'); +const swaggerUi = require('../lib/swagger-ui'); /** @@ -31,21 +32,23 @@ /** * 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('/', (req, res, next) => { - if (!{}.hasOwnProperty.call(req.query || {}, '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; diff --git a/test/features/app/app.js b/test/features/app/app.js index dbafca1..c06d5ce 100644 --- a/test/features/app/app.js +++ b/test/features/app/app.js @@ -5,6 +5,10 @@ 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() { @@ -76,5 +80,4 @@ assert.deepEqual(contentEncoding, undefined, 'Did not expect gzipped contents!'); }); }); - }); diff --git a/test/features/app/spec.js b/test/features/app/spec.js index c5abfb0..6b31ebc 100644 --- a/test/features/app/spec.js +++ b/test/features/app/spec.js @@ -2,13 +2,14 @@ const preq = require('preq'); -const assert = require('../../utils/assert'); -const server = require('../../utils/server'); -const dateUtil = require('../../../lib/dateUtil'); -const pad = dateUtil.pad; +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'); + +const dateUtil = require('../../../lib/dateUtil'); +const pad = dateUtil.pad; const Ajv = require('ajv'); const baseUri = `${server.config.uri}en.wikipedia.org/v1/`; @@ -18,6 +19,10 @@ const monthDayStr1 = `${pad(d1.getUTCMonth() + 1)}/${pad(d1.getUTCDate())}`; const dateStr2 = `${d2.getUTCFullYear()}/${pad(d2.getUTCMonth() + 1)}/${pad(d2.getUTCDate())}`; +if (!server.stopHookAdded) { + server.stopHookAdded = true; + after(() => server.stop()); +} function staticSpecLoad() { @@ -62,8 +67,9 @@ try { uri.expand(Object.assign({}, defParams, ex.request.params || {})); } catch (e) { - const msg = `Route ${pathStr}, example ${ex.title}: missing parameter: ${e.message}`; - throw new Error(msg); + throw new Error( + `Route ${pathStr}, example ${idx} (${ex.title}): missing parameter: ${e.message}` + ); } }); @@ -119,9 +125,7 @@ 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 || {} @@ -220,8 +224,9 @@ } // check that the body type is the same if (expRes.body.constructor !== res.body.constructor) { - const msg = `Expected body type ${expRes.body.constructor} but got ${res.body.constructor}`; - throw new Error(msg); + throw new Error( + `Expected body type ${expRes.body.constructor} but got ${res.body.constructor}` + ); } // compare the bodies diff --git a/test/features/info/info.js b/test/features/info/info.js index c68164a..9eaeb82 100644 --- a/test/features/info/info.js +++ b/test/features/info/info.js @@ -5,6 +5,10 @@ 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() { @@ -65,6 +69,5 @@ assert.notDeepEqual(res.body.home, undefined, 'No home field returned!'); }); }); - }); diff --git a/test/index.js b/test/index.js index a8c4173..8c26dad 100644 --- a/test/index.js +++ b/test/index.js @@ -2,3 +2,9 @@ // Run jshint as part of normal testing require('mocha-jshint')(); +require('mocha-eslint')([ + 'lib', + 'routes' +], { + timeout: 10000 +}); diff --git a/test/utils/server.js b/test/utils/server.js index b4d0982..ac69769 100644 --- a/test/utils/server.js +++ b/test/utils/server.js @@ -1,6 +1,10 @@ 'use strict'; +// mocha defines to avoid JSHint breakage +/* global describe, it, before, beforeEach, after, afterEach */ + + // Default to recording+replaying http fixtures, // only if we are not running in Docker if (process.env.IN_DOCKER) { @@ -39,7 +43,7 @@ // make a deep copy of it for later reference const origConfig = extend(true, {}, config); -let stop = () => { return BBPromise.resolve(); }; +module.exports.stop = () => { return BBPromise.resolve(); }; let options = null; const runner = new ServiceRunner(); @@ -49,18 +53,23 @@ _options = _options || {}; if (!assert.isDeepEqual(options, _options)) { - console.log('server options changed; restarting'); // eslint-disable-line no-console - return stop().then(() => { + 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(() => { - stop = function() { + .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(() => { - stop = function() { return BBPromise.resolve(); }; + module.exports.stop = function() { + return BBPromise.resolve(); + }; }); }; return true; @@ -69,7 +78,6 @@ } else { return BBPromise.resolve(); } - } module.exports.config = config; -- To view, visit https://gerrit.wikimedia.org/r/390878 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: Id6a2fe9c8656ce9947e2b9936275a40db704e402 Gerrit-PatchSet: 5 Gerrit-Project: mediawiki/services/mobileapps Gerrit-Branch: master Gerrit-Owner: Mholloway <mhollo...@wikimedia.org> Gerrit-Reviewer: BearND <bsitzm...@wikimedia.org> Gerrit-Reviewer: Fjalapeno <cfl...@wikimedia.org> Gerrit-Reviewer: Jdlrobson <jrob...@wikimedia.org> Gerrit-Reviewer: Mholloway <mhollo...@wikimedia.org> Gerrit-Reviewer: Mhurd <mh...@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