diff --git a/.eslintrc.js b/.eslintrc.js deleted file mode 100644 index aefe031..0000000 --- a/.eslintrc.js +++ /dev/null @@ -1,3 +0,0 @@ -module.exports = { - extends: 'kartotherian', -}; diff --git a/.eslintrc.yml b/.eslintrc.yml new file mode 100644 index 0000000..bc67459 --- /dev/null +++ b/.eslintrc.yml @@ -0,0 +1 @@ +extends: 'eslint-config-kartotherian' \ No newline at end of file diff --git a/.jshintignore b/.jshintignore deleted file mode 100644 index a4c38eb..0000000 --- a/.jshintignore +++ /dev/null @@ -1,4 +0,0 @@ -coverage -node_modules -test -static diff --git a/.jshintrc b/.jshintrc deleted file mode 100644 index afaeeec..0000000 --- a/.jshintrc +++ /dev/null @@ -1,29 +0,0 @@ -{ - "predef": [ - "ve", - "setImmediate", - "QUnit", - "Map", - "Set" - ], - "bitwise": true, - "laxbreak": true, - "curly": true, - "eqeqeq": true, - "immed": true, - "latedef": true, - "newcap": true, - "noarg": true, - "noempty": true, - "nonew": true, - "regexp": false, - "undef": true, - "strict": true, - "trailing": true, - "smarttabs": true, - "multistr": true, - "node": true, - "nomen": false, - "loopfunc": true, - "esnext": true -} diff --git a/Gruntfile.js b/Gruntfile.js deleted file mode 100644 index 0167a9a..0000000 --- a/Gruntfile.js +++ /dev/null @@ -1,35 +0,0 @@ -/* eslint-env node */ -module.exports = function Gruntfile(grunt) { - grunt.loadNpmTasks('grunt-contrib-watch'); - grunt.loadNpmTasks('grunt-eslint'); - grunt.loadNpmTasks('grunt-mocha-test'); - - grunt.initConfig({ - watch: { - scripts: { - files: ['**/*.js'], - tasks: ['test'], - }, - }, - eslint: { - code: { - src: [ - '**/*.js', - '!node_modules/**', - ], - }, - }, - mochaTest: { - test: { - options: { - reporter: 'spec', - }, - src: ['test/**/*.js'], - }, - }, - }); - - grunt.registerTask('lint', ['eslint']); - grunt.registerTask('test', ['lint', 'mochaTest']); - grunt.registerTask('default', 'test'); -}; diff --git a/app.js b/app.js index ec41743..7e538b4 100644 --- a/app.js +++ b/app.js @@ -7,6 +7,8 @@ const fs = BBPromise.promisifyAll(require('fs')); const sUtil = require('./lib/util'); const packageInfo = require('./package.json'); const yaml = require('js-yaml'); +const addShutdown = require('http-shutdown'); +const path = require('path'); /** @@ -93,6 +95,9 @@ function initApp(options) { next(); }); + // set up the user agent header string to use for requests + app.conf.user_agent = app.conf.user_agent || app.info.name; + // disable the X-Powered-By header app.set('x-powered-by', false); // disable the ETag header - users should provide them! @@ -113,21 +118,23 @@ function initApp(options) { * @param {Application} app the application object to load routes into * @returns {bluebird} a promise resolving to the app object */ -function loadRoutes(app) { - // get the list of files in routes/ - return fs.readdirAsync(`${__dirname}/routes`).map(fname => BBPromise.try(() => { - // ... and then load each route - // but only if it's a js file - if (!/\.js$/.test(fname)) { - return undefined; +function loadRoutes(app, dir) { + // recursively load routes from .js files under routes/ + /* eslint-disable consistent-return,no-param-reassign */ + return fs.readdirAsync(dir).map(fname => BBPromise.try(() => { + const resolvedPath = path.resolve(dir, fname); + const isDirectory = fs.statSync(resolvedPath).isDirectory(); + if (isDirectory) { + loadRoutes(app, resolvedPath); + } else if (/\.js$/.test(fname)) { + // import the route file + // eslint-disable-next-line global-require,import/no-dynamic-require + const route = require(`${dir}/${fname}`); + return route(app); } - // import the route file - // eslint-disable-next-line import/no-dynamic-require,global-require - const route = require(`${__dirname}/routes/${fname}`); - return route(app); }).then((route) => { if (route === undefined) { - return; + return undefined; } // check that the route exports the object we need if ( @@ -138,21 +145,27 @@ function loadRoutes(app) { ) { throw new TypeError(`routes/${fname} does not export the correct object!`); } - // wrap the route handlers with Promise.try() blocks - sUtil.wrapRouteHandlers(route.router); - // determine the path prefix - let prefix = ''; + // normalise the path to be used as the mount point + if (route.path[0] !== '/') { + route.path = `/${route.path}`; + } + if (route.path[route.path.length - 1] !== '/') { + route.path = `${route.path}/`; + } if (!route.skip_domain) { - prefix = `/:domain/v${route.api_version}`; + 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(prefix + route.path, route.router); + app.use(route.path, route.router); })).then(() => { // catch errors sUtil.setErrorHandler(app); // route loading is now complete, return the app object return BBPromise.resolve(app); }); + /* eslint-enable consistent-return,no-param-reassign */ } @@ -166,17 +179,26 @@ function createServer(app) { // attaches the app to it, and starts accepting // incoming client requests let server; - return new BBPromise(((resolve) => { + return new BBPromise((resolve) => { server = http.createServer(app).listen( app.conf.port, app.conf.interface, resolve ); - })).then(() => { + 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; }); } @@ -189,6 +211,10 @@ function createServer(app) { */ module.exports = function module(options) { return initApp(options) - .then(loadRoutes) - .then(createServer); + .then(app => loadRoutes(app, `${__dirname}/routes`)) + .then((app) => { + // serve static files from static/ + app.use('/static', express.static(`${__dirname}/static`)); + return app; + }).then(createServer); }; diff --git a/doc/README.md b/doc/README.md deleted file mode 100644 index d837f0a..0000000 --- a/doc/README.md +++ /dev/null @@ -1,14 +0,0 @@ -This directory contains the documentation for the service template, aimed at -getting you started up and running quickly. - -The documentation should be read in the following order: - -1. [Short API design practices](api-design.md) -2. [Service template overview](template.md) -3. [Configuration](config.md) -4. [Useful Commands](commands.md) -5. [Coding Guide](coding.md) -6. [Deployment](deployment.md) - -Have fun while creating RESTful API services! - diff --git a/doc/api-design.md b/doc/api-design.md deleted file mode 100644 index 20f2e5b..0000000 --- a/doc/api-design.md +++ /dev/null @@ -1,83 +0,0 @@ -# API Design - -Before you start coding your service, you need to think hard about your API's -design, especially for a public service exposing its API. Below are a couple of -practices you should follow. - -- [Statelessness](#statelessness) -- [Versioning](#versioning) -- [Hierarchical URI Layout](#hierarchical-uri-layout) -- [HTTP Verbs](#http-verbs) -- [Documentation](#documentation) -- [See Also](#see-also) - -## Statelessness - -RESTful API services should be -[stateless](https://en.wikipedia.org/wiki/Service_statelessness_principle), since -they are conceptually modelled around *resources* (as opposed to *systems*). -Accordingly, your service should take actions depending on assumptions about the -caller or the current state of the service's process (e.g. that the user is -logged in, that they are allowed to modify a resource, etc.) - -## Versioning - -You should always version all of your API. Always. Period. Applications depend on -its stability and invariance. It is tolerable to add endpoints to an API -version, but removing or modifying existing ones is not an option. Thus, -versioning provides an easy way for you to improve upon the API while avoiding -third-party application breakage. The template enforces this practice by -requiring all of your [route files](../routes/) specify the API version. - -## Hierarchical URI Layout - -Use a hierarchical URI layout that is intuitive and makes sense. Grouping -endpoints under a common URI prefix allows both you and the future API consumer -to reason about the API. As an example, consider -[RESTBase](https://www.mediawiki.org/wiki/RESTBase)'s API layout: - -``` -/{domain} - -- /v1 - |- /page - | |- /title - | |- /html - | |- /data-parsoid - | -- /revision - -- /transform - |- /html/to/wikitext - |- /wikitext/to/html - -- /html/to/html -``` - -The API is grouped in two *sections* - `page` and `transform`. The former -exposes endpoints dealing with Wiki pages, while the latter comprises endpoints -transforming one format to another. - -## HTTP Verbs - -There are many [HTTP -verbs](http://www.w3.org/Protocols/rfc2616/rfc2616-sec9.html) you can use and -expose in your API. Use them appropriately. Especially, **do not allow GET -requests to modify** any type of content. - -## Documentation - -Document your API meticulously, and keep it up to date with the code. Remember -that API's are meant to be consumed by external applications, whose developers -most often do not know the internal workings of your stack. A good starting -point is to look into [Swagger](https://github.com/swagger-api/swagger-spec), a -specification for API declaration and documentation from which nice, demo-able -documentation such as [this](http://rest.wikimedia.org/en.wikipedia.org/v1/?doc) -can be automatically generated. - -## See Also - -The above is just a short list of things you should think about when designing -your API. Here are some resources you might find useful at this step: - -- https://github.com/Wikia/guidelines/tree/master/APIDesign -- https://restful-api-design.readthedocs.org/en/latest/ -- http://www.vinaysahni.com/best-practices-for-a-pragmatic-restful-api -- http://www.thoughtworks.com/insights/blog/rest-api-design-resource-modeling - diff --git a/doc/coding.md b/doc/coding.md deleted file mode 100644 index 27aa85e..0000000 --- a/doc/coding.md +++ /dev/null @@ -1,350 +0,0 @@ -# Coding Guide - -Let's get started! - -- [Route Set-up](#route-set-up) -- [Routes](#routes) -- [Promises](#promises) - - [I/O](#io) - - [External Requests](#external-requests) -- [Error Handling](#error-handling) -- [Logging and Metrics](#logging-and-metrics) - - [Logging](#logging) - - [Metrics Collection](#metrics-collection) -- [Test Cases](#test-cases) - -## Route Set-up - -All of the routes are read from the [routes directory](../routes) and are -automatically mounted on start-up. The first step is to create a new route file -by copying the [route template](../routes/empty.js.template): - -```bash -$ cd routes -$ cp empty.js.template people.js -``` - -Now, open `people.js` in your favourite editor. The first thing you need to -decide is the mount path for the routes contained in the file and the API -version the route belongs to. Let's say that this file will contain routes -pertaining to famous people, so a path like `/people/` makes sense here. -Obviously, the API version is going to be `1`. Change lines 32 - 36 to reflect -this: - -```javascript -return { - path: '/people', - api_version: 1, - router: router -}; -``` - -This causes all of the routes you create in `people.js` to be mounted on -`/{domain}/v1/people/`, where `{domain}` represents the sought domain (such as -`en.wikipedia.org`, `www.mediawiki.org`, etc.). - -## Routes - -Creating routes is accomplished by calling `router.METHOD(path, handlerFunction)` -where `METHOD` is the HTTP verb you want to create the route for (`get`, `put`, -`post`, etc.), and `handlerFunction` is the callback function called when the -relative path `path` is matched. We are now ready to set up our first route. -Replace line 23 with the following code: - -```javascript -router.get('/:name', function(req, res) { - - res.status(200).json({ - name: decodeURIComponent(req.params.name) - }); - -}); -``` - -The route's path is `:name`, which signifies a variable path. In this case, it -is the name of the person the request is about. Thus, both -`/people/Albert_Einstein` and `/people/David_Lynch` will match the route. The -callback's body is rather simple: we set the response's status to `200` and send -back a JSON containing the person's name. To learn more about routes and their -various options, read Express.js' [routing -guide](http://expressjs.com/guide/routing.html). - -## Promises - -The service template includes the [bluebird -module](https://github.com/petkaantonov/bluebird) for handling asynchronous -patterns via promises. Prime examples of when they should be used are performing -external requests or I/O actions. Promises allow the service process not to -block on them and continue serving other requests until the action is completed. - -### I/O - -Coming back to our example route, let's say that we want to serve a simple HTML -document on the endpoint `/people/:name/about`. To do so, first we need to -require and *promisify* the `fs` module. Put this line in the header of your -routes file (right below line 6): - -```javascript -var fs = BBPromise.promisifyAll(require('fs')); -``` - -This creates additional functions, which are *promisified* versions of the -original ones exported by the `fs` module. Henceforth, we can read a file either -using the built-in `fs.readFile()` or its promise-aware counterpart -`fs.readFileAsync()`. - -Armed with this knowledge, we can now easily create a route handler: - -```javascript -router.get('/:name/about', function(req, res) { - - // read the file - return fs.readFileAsync(__dirname + '/../static/index.html') - // and then send back its contents - .then(function(src) { - res.status(200).type('html').send(src); - }); - -}); -``` -As you can see, promises allow us to specify chained actions in a natural way -(using the `.then()` continuation pattern). Note that, when using promises in -services derived from this template it is important that you `return` the -promise to the caller. Doing so allows the template's framework to automatically -handle any possible errors during the promise's execution. - -### External Requests - -One other area where promises come in handy is making external requests. Suppose -we want to serve the latest news about a person from -[Wikinews](http://www.wikinews.org). The template includes the -[preq](https://github.com/gwicke/preq) -- a module promisifying the popular -[request](https://github.com/request/request) module -- which we can use -right away: - -```javascript -router.get('/:name/news/:lang?', function(req, res) { - - // set the language if not set - var lang = req.params.lang || 'en'; - - // get the news - return preq.get({ - uri: 'https://' + lang + '.wikinews.org/wiki/' - + encodeURIComponent(req.params.name) - }).then(function(wnRes) { - res.status(200).type('html').send(wnRes.body); - }); - -}); -``` - -## Error Handling - -As mentioned earlier, the template is capable of automatically handling errors -for you. However, you might want to take matters into your own hands in some -occasions. The template provides a convenient `HTTPError` object class which you -can use. - -Let's revise the handler for the `/people/:name/about` route. It does not seem -to be very useful, as it returns the same content for any given name. We would -like it to return content relevant to the person whose name was specified in the -request URI by looking up the file `/static/name.html`. If the file does not -exist, a `404` should be returned to the caller. - -```javascript -router.get('/:name/about', function(req, res) { - - return fs.readFileAsync(__dirname + '/../static/' - + encodeURIComponent(req.params.name) + '.html') - .then(function(src) { - res.status(200).type('html').send(src) - }).catch(function(err) { - throw new HTTPError({ - status: 404, - type: 'not_found', - title: 'Not Found', - detail: 'No information could be found on ' + req.params.name - }); - }); - -}); -``` - -Note that you can also attach additional debug information to the `HTTPError` -object to help you track down bugs. This information is going to be logged, but -will not reach the client, thus ensuring no sensitive information is leaked -unintentionally. To do so, simply add any property you deem important when -creating / throwing the error. - -## Logging and Metrics - -Logging and metrics collection is supported out of the box via -[service-runner](https://github.com/wikimedia/service-runner). They are exposed -in route handler files via the `req.logger` and `app.metrics` objects. - -### Logging - -To log something, simply use `req.logger.log(level, what)`. The logger itself is -a [bunyan](https://github.com/trentm/node-bunyan) wrapper, and thus supports the -following levels: - -- `trace` -- `debug` -- `info` -- `warn` -- `error` -- `fatal` - -Additionally, it is good practice to attach a component name to the log level as -it eases log indexing and filtering later in production. For example, if a log -entry has the `debug` level and pertains to one of our example routes, the log -level could be set to `debug/people`. The `what` portion of the log entry can be -either a string message, or any *stringifiable* object. As an example, let's -log the person's name given to the `/people/:name/about` route and the file name -that is going to be looked up: - -```javascript -router.get('/:name/about', function(req, res) { - - var info = { - name: req.params.name, - path: __dirname + '/../static/' - + encodeURIComponent(req.params.name) + '.html' - }; - - req.logger.log('debug/people/about', info); - - return fs.readFileAsync(info.path) - .then(function(src) { - res.status(200).type('html').send(src) - }).catch(function(err) { - throw new HTTPError({ - status: 404, - type: 'not_found', - title: 'Not Found', - detail: 'No information could be found on ' + info.name - }); - }); - -}); -``` - -As you can see, the request object (`req`) has an additional property - -`req.logger`, which allows you to log messages and objects in the context of the -current request. To do so, it attaches a unique *request ID* to each logged -information. If you would like to log context-free information, you can use the -`app.logger` object instead, even though that is not recommended. - -### Metrics Collection - -Collecting metrics is a great way to have insights into the overall health and -performance of your service. When using the template, this is as easy as calling -one of the following methods: - -- `app.metrics.timing` -- `app.metrics.increment` -- `app.metrics.decrement` -- `app.metrics.histogram` -- `app.metrics.gauge` -- `app.metrics.unique` - -How can one collect them? Let's show it on `/people/:name/news`. This route uses -an external request to complete its action, which means that you have little -control over your service's response time, as it is dominated by the request to -Wikinews. Two interesting metrics that we can collect here (and that directly -affect the service's response time) are the external request's response time and -the size of its response. We can measure the former with `app.metrics.timing()` -and the latter with `app.metrics.histogram()`. Additionally, it interesting to -see the distribution of languages, which can be achieved with -`app.metrics.unique()`. - -```javascript -router.get('/:name/news/:lang?', function(req, res) { - - // set the language if not set - var lang = req.params.lang || 'en'; - - // count the language occurrence - app.metrics.unique('people.news.lang', lang); - // start measuring the time - var startTime = Date.now(); - - // get the news - return preq.get({ - uri: 'https://' + lang + '.wikinews.org/wiki/' - + encodeURIComponent(req.params.name) - }).then(function(wnRes) { - // external request done, report the request time - app.metrics.timing('people.news.time', Date.now() - startTime); - // also report the payload's size - app.metrics.histogram('people.news.size', wnRes.body.length); - res.status(200).type('html').send(wnRes.body); - }); - -}); -``` -For more information on the available methods, see the [service-runner -documentation](https://github.com/wikimedia/service-runner#metric-reporting). - -## Test Cases - -The service needs to thoroughly tested since other services and clients are -going to depend on it. The template uses -[mocha](https://github.com/mochajs/mocha) for test execution and provides some -useful utility functions in [test/utils](../test/utils). - -To create a test suite for our example routes, create the `people` directory in -`/test/features/` and two files inside of it: `about.js` and `news.js`. These -will test the example routes. Let's start with `about.js`: - -```javascript -'use strict'; - - -// mocha defines to avoid JSHint breakage -/* global describe, it, before, beforeEach, after, afterEach */ - - -var preq = require('preq'); -var assert = require('../../utils/assert.js'); -var server = require('../../utils/server.js'); - - -describe('people - about', function() { - - this.timeout(20000); - - before(function () { return server.start(); }); - - // common URI prefix - var uri = server.config.uri + 'en.wikipedia.org/v1/people/'; - - it('get HTML for index', function() { - return preq.get({ - uri: uri + 'index/about' - }).then(function(res) { - // check the status - assert.status(res, 200); - // check the returned Content-Type header - assert.contentType(res, 'text/html'); - // inspect the body - assert.notDeepEqual(res.body, undefined, 'No body returned!'); - }); - }); - - it('fail for a non-existent person', function() { - return preq.get({ - uri: uri + 'Walt_Disney/about' - }).then(function(res) { - // if we are here, no error was thrown, not good - throw new Error('Expected an error to be thrown, got status: ', res.status); - }, function(err) { - // inspect the status - assert.deepEqual(err.status, 404); - }); - }); - -}); -``` - diff --git a/doc/commands.md b/doc/commands.md deleted file mode 100644 index 01b6c5a..0000000 --- a/doc/commands.md +++ /dev/null @@ -1,101 +0,0 @@ -# Useful Commands - -- [npm 101](#npm-101) -- [Service-related Tasks](#service-related-tasks) -- [Docker](#docker) - -## npm 101 - -[npm](https://www.npmjs.com/) is the package manager for Node.js modules. It is -used for managing and publishing modules. - -The template (and your future service) needs its dependencies to be present. -Install them with: - -``` -npm install -``` - -Sometimes the configuration can get a bit messed up you may experience strange -*npm*-related errors when running your service. The remedy is: - -``` -rm -rf node_modules -npm install -``` - -If you need to add a dependency, this will install it and add it your -`package.json`: - -``` -npm install --save -``` - -## Service-related Tasks - -The template comes with some handy `npm` tasks. To start your service based on -the configuration in `config.yaml`, use simply: - -``` -npm start -``` - -Starting unit tests is as easy as: - -``` -npm test -``` - -A code coverage utility is also available: - -``` -npm run-script coverage -``` - -Once the script finishes, open up `coverage/lcov-report/index.html` which will -show you detailed reports about which lines of code have been covered by the -unit tests. - -## Docker - -Included in the template is also a Dockerfile, allowing you to run and test your -service in a production-like environment inside of a Docker container. You need -to have [docker](https://www.docker.com/) installed if you are on a Linux host, -or [boot2docker](http://boot2docker.io/) in case of OSX/Windows hosts. - -To start your service in the container, execute: - -``` -npm run-script docker-start -``` - -The first time you run it, it takes a while as the script automatically builds -the full image and then starts the service. - -If you want to test your service instead, use: - -``` -npm run-script docker-test -``` - -Similarly, to run code coverage, run: - -``` -npm run-script docker-cover -``` - -*Note:* On Linux hosts, running `docker` requires superuser rights, so you may -need to prefix the commands with `sudo`. If you are on a Ubuntu box, you may -circumvent that by adding yourself to the `docker` group: - -``` -sudo gpasswd -a docker -``` - -After you log out completely and log back in, you should be able to run the -above scripts without resorting to `sudo`. - -## Deployment - -See [this document](deployment.md) for how to get ready to deploy your service. - diff --git a/doc/config.md b/doc/config.md deleted file mode 100644 index e2009f8..0000000 --- a/doc/config.md +++ /dev/null @@ -1,49 +0,0 @@ -# Configuration - -The first thing you should configure is the service's general information (name, -description, etc.). Open up [`package.json`](../package.json) and change (at -least) the following fields: - -- `name` -- `version` -- `description` -- `repository/url` -- `keywords` -- `author` -- `contributors` -- `licence` -- `bugs` -- `homepage` - -Now change the service's name in [`config.dev.yaml`](../config.dev.yaml#L26) and -[`config.prod.yaml`](../config.prod.yaml#L26). While you are there, you might -want to look at and play with other configuration parameters, such as: - -- `num_workers` - the number of workers to start; some special values are: - - `0` will not do any forking, but run the service in the master process - - `ncpu` will spawn as many worker processes as there are CPU cores on the - host -- `worker_heap_limit_mb` - the maximum amount of memory (in MB) a worker's heap - can have -- `logging` and `metrics` - the configuration for logging and metrics facilities -- `services` - the block instructing the master process which services to start; - there can be more than one service, if, e.g., your service depends on another - Node.js service being present; each service has further the following - information: - - `name` - the service's name - - `module` - the module starting the service; if not given, the service's name - is used instead - - `conf` - the configuration object passed directly to the service; settings - to consider (remember to update them in both - [`config.dev.yaml`](../config.dev.yaml) as well as - [`config.prod.yaml`](../config.prod.yaml)): - - `port` - the port to start the service on (default: `8888`) - - `interface` - where to bind the service's server (default: `0.0.0.0`) - - you may add here any other configuration options needed by your service, - as long as it is [valid YAML](http://www.yaml.org/spec/1.2/spec.html); - these will be accessible via the `app.conf` object - -For more information on configuration possibilities, take a look at the -[service-runner -documentation](https://github.com/wikimedia/service-runner#config-loading). - diff --git a/doc/deployment.md b/doc/deployment.md deleted file mode 100644 index ae0b2f4..0000000 --- a/doc/deployment.md +++ /dev/null @@ -1,190 +0,0 @@ -# Deployment - -Getting your service ready to be deployed on WMF production machines involves -several tasks. This document explains the steps needed to get started and how to -keep your deployable copy up-to-date. - -## Repositories - -Because Node.js services use npm dependencies which can be binary, these need to -be pre-built. Therefore, two repositories are needed; one for the source code of -your service, and the other, so-called *deploy* repository. Both should be -available as WM's Gerrit repositories with the paths -*mediawiki/services/your-service-name* and -*mediawiki/services/your-service-name/deploy*. When [requesting -them](https://www.mediawiki.org/wiki/Git/New_repositories/Requests) ask for the -former to be a clone of [the service -template](https://github.com/wikimedia/service-template-node) and the latter to -be empty. - -It is important to note that the deploy repository is only to be updated -directly before (re-)deploying the service, and not on each patch merge entering -the *master* branch of the regular repository. In other words, **the deploy -repository mirrors the code deployed in production at all times**. - -The remainder of the document assumes these two repositories have been created -and that you have cloned them using your Gerrit account, i.e. not anonymously, -with the following outline: - -``` -~/code/ - |- your-service - -- deploy -``` - -Furthermore, it is assumed that you have initialised the deploy repository: - -```bash -$ cd ~/code/deploy -$ git review -s -$ touch README.md -$ git add README.md -$ git commit -m "Initial commit" -$ git push -u origin master # or git review -R if this fails -# go to Gerrit and +2 your change, if needed and then: -$ git pull -``` - -Finally, if you haven't yet done so, do [basic service -configuration](config.md). - -The remainder of the document refers to these two repositories as the *source -repository* and the *deploy repository*, respectively. - -## Configuration - -The service template includes an automation script which updates the deploy -repository, but it needs to be configured properly in order to work. - -### package.json - -The first part of the configuration involves keeping your source repository's -`package.json` updated. Look for its [deploy stanza](../package.json#L49). -Depending on the exact machine on which your service will be deployed, you may -need to set `target` to either `ubuntu` or `debian`. - -If you want to specify a version of Node.JS, different from the official distribution -package, set the value of the `node` stanza to the desired version, following -[nvm](https://github.com/creationix/nvm) versions naming. -To explicitly force official distribution package, `"system"` version can be used. - -The important thing is keeping the `dependencies` field up to date at all times. -There you should list all of the extra packages that are needed in order to -build the npm module dependencies. The `_all` field denotes packages which -should be installed regardless of the target distribution, but you can add -other, distribution-specific package lists, e.g.: - -```javascript -"deploy": { - "target": "ubuntu", - "node": "system", - "dependencies": { - "ubuntu": ["pkg1", "pkg2"], - "debian": ["pkgA", "pkgB"], - "_all": ["pkgOne", "pkgTwo"] - } -} -``` - -In this example, with the current configuration, packages *pkg1*, *pkg2*, -*pkgOne* and *pkgTwo* are going to be installed before building the -dependencies. If, instead, the target is changed to `debian`, then *pkgA*, -*pkgB*, *pkgOne* and *pkgTwo* are selected. - -As a rule of thumb, **whenever you need to install extra packages into your -development environment for satisfying node module dependencies, add them to -*deploy.dependencies* to ensure the successful build and update of the deploy -repository**. - -### Local git - -The script needs to know where to find your local copy of the deploy repository. -To that end, when in your source repository, run: - -``` -git config deploy.dir /absolute/path/to/deploy/repo -``` - -Using the aforementioned local outline, you would type: - -``` -git config deploy.dir /home/YOU/code/deploy -``` - -The source repository is itself a submodule of the deploy repository. If its -name as specified in `package.json`'s `name` field does not match the actual -repository's name in Gerrit, run: - -``` -git config deploy.name name_in_gerrit -``` - -That will make the system look for the repository -`mediawiki/services/name_in_gerrit` when checking it out in the deploy -repository. If, however, you do not use MediaWiki's Gerrit installation to host -your repository, you can specify a different one using the `deploy.submodule` -configuration directive by supplying the full remote reference URL: - -``` -git config deploy.submodule https://github.com/your_team/repo_name -``` - -The deploy-repo builder script assumes the name of the remote to check out in -the deploy repository is `origin`. An alternative name can be configured by -invoking (in the source repository): - -``` -git config deploy.remote deploy_repo_remote_name -``` - -## Testing - -Before updating the deploy repository you need to make sure your configuration -works as expected. To do that, in your source repository run: - -``` -./server.js docker-test -``` - -The script will build a new Docker image, install the needed packages and npm -dependencies and run the test suite. Tweak your code and configuration until -everything works as expected (and commit those changes). - -## Update - -The final step is updating the deploy repository. First. make sure that your -source repository has got the latest dependencies installed: - -``` -rm -rf node_modules/ && npm install -``` - -Update the deploy repository by running from the source repository: - -``` -./server.js build --deploy-repo -``` - -The script will: -- create the proper deploy repository outline -- fetch the updates -- ensure the submodule is present -- update the submodule -- build the npm dependencies -- commit the changes with a pretty-formatted message - -There is also a handy shortcut for sending the patch to Gerrit immediately. To -do so, add the `--review` argument to the call: - -``` -./server.js build --deploy-repo --review -``` - -Note that if no changes were made to the source repository, the script aborts -its execution. If, nevertheless, you need to rebuild the dependencies, you can -do so using: - -``` -./server.js build --deploy-repo --force -``` - diff --git a/doc/template.md b/doc/template.md deleted file mode 100644 index f9d28ad..0000000 --- a/doc/template.md +++ /dev/null @@ -1,67 +0,0 @@ -# Service Template Overview - -This service template allows you to quickly dive into coding your own RESTful API -service. - -- [Stack](#stack) -- [Repository Outline](#repository-outline) - -## Stack - -The template makes use the following components: - -- [service-runner](https://github.com/wikimedia/service-runner) -- [express.js](http://expressjs.com/) -- [Bluebird Promises](https://github.com/petkaantonov/bluebird) -- [mocha](http://mochajs.org/) -- [istanbul](https://github.com/gotwarlost/istanbul) -- [docker](https://www.docker.com/) - -Everything begins and ends with *service-runner*. It is the supervisor in charge -of starting the service and controlling its execution. It spawns worker -processes, each of which accepts and handles connections. If some workers use -too much heap memory, they are restarted. Additionally, it provides your service -with configurable logging and metrics facilities. - -When it comes to request handling, *express.js* and *Bluebird* take the centre -stage. *express.js* is in charge of receiving the requests, routing and -dispatching them to the correct handlers and send responses back to the clients. -*Bluebird* comes into play when there are actions which warrant asynchronous -processing (such as reading files, dispatching requests to external resources, -etc.). You can find example route handlers constructing the response both -synchronously and asynchronously in this template's [routes](../routes/) -directory. - -Finally, testing is an important aspect of service programming, not only for -their creators, but also testers (think CI) and consumers. The template uses -*mocha* for carrying out the testing, and *istanbul* for reporting code coverage -with tests. There are quite a few tests [available](../test/) for you to check -out. - -The WMF is in the process of switching its production servers to Debian Jessie. -As people developing services might use different platforms, the template -provides also a Dockerfile, with which one can execute their service inside a -container running the production OS. - -## Repository Outline - -Below is a simplified repository outline listing the important files/directories -for service development. - -- [`package.json`](../package.json) - the file containing the service's name and - dependencies -- [`config.dev.yaml`](../config.dev.yaml) and - [`config.prod.yaml`](../config.prod.yaml) - contain development and production - configuration settings for the service -- [`server.js`](../server.js) - the service's starter script -- [`app.js`](../app.js) - contains the application declaration and loading logic -- [`routes`](../routes/) - contains the definitions of the loaded routes; this - is where most of your coding is to take place -- [`lib/util.js`](../lib/util.js) - contains some utility functions and classes -- [`static`](../static/) - this is where served static files go (HTML, CSS, - client-side JS, etc.) -- [`test`](../test/) - contains the test files for the example routes in the - template; you should add your own here -- docker script - a utility script building the service's docker image and - starting the container, now part of [service-runner](wikimedia/service-runner) - diff --git a/lib/swagger-ui.js b/lib/swagger-ui.js new file mode 100644 index 0000000..727775e --- /dev/null +++ b/lib/swagger-ui.js @@ -0,0 +1,78 @@ +const BBPromise = require('bluebird'); +const fs = BBPromise.promisifyAll(require('fs')); +const path = require('path'); +const HTTPError = require('../lib/util.js').HTTPError; // eslint-disable-line prefer-destructuring + + +// Swagger-ui helpfully exporting the absolute path of its dist directory +const docRoot = `${require('swagger-ui').dist}/`; // eslint-disable-line global-require + +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) => { + /* eslint-disable no-param-reassign */ + if (reqPath === '/index.html') { + body = body.toString() + .replace(/((?:src|href)=['"])/g, '$1?doc&path=') + // Some self-promotion + .replace( + /` + ) + .replace(/[^<]*<\/title>/, `<title>${app.info.name}`) + // 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/'); + } + /* eslint-enable no-param-reassign */ + + 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 97ee2fc..88597d7 100644 --- a/lib/util.js +++ b/lib/util.js @@ -1,7 +1,4 @@ - - const BBPromise = require('bluebird'); -const util = require('util'); const express = require('express'); const uuid = require('cassandra-uuid'); const bunyan = require('bunyan'); @@ -10,47 +7,36 @@ const bunyan = require('bunyan'); /** * Error instance wrapping HTTP error responses */ -function HTTPError(response) { - let key; - let normalizedResponse = response; - - Error.call(this); - Error.captureStackTrace(this, HTTPError); +class HTTPError extends Error { + constructor(response) { + super(); + Error.captureStackTrace(this, HTTPError); - if (response.constructor !== Object) { - // just assume this is just the error message - normalizedResponse = { - status: 500, - type: 'internal_error', - title: 'InternalError', - detail: response, - }; - } - - this.name = this.constructor.name; - this.message = `${response.status}`; - if (normalizedResponse.type) { - this.message += `: ${response.type}`; - } + if (response.constructor !== Object) { + // just assume this is just the error message + response = { // eslint-disable-line no-param-reassign + status: 500, + type: 'internal_error', + title: 'InternalError', + detail: response, + }; + } - // TODO: Replace this for-in statement with hasOwnProperty with a better loop - // like Object.keys and a binding for context 'this' - // eslint-disable-next-line no-restricted-syntax - for (key in normalizedResponse) { - if (Object.prototype.hasOwnProperty.call(normalizedResponse, key)) { - this[key] = normalizedResponse[key]; + this.name = this.constructor.name; + this.message = `${response.status}`; + if (response.type) { + this.message += `: ${response.type}`; } + + Object.assign(this, response); } } -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) { const ret = { @@ -77,10 +63,9 @@ function reqForLog(req, whitelistRE) { /** - * 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) { const ret = bunyan.stdSerializers.err(err); @@ -97,9 +82,8 @@ function errForLog(err) { } /** - * Generates a unique request ID - * - * @return {String} the generated request ID + * Generates a unique request ID. + * @return {!string} the generated request ID */ function generateRequestId() { return uuid.TimeUuid.now().toString(); @@ -111,17 +95,36 @@ function generateRequestId() { * promised try blocks so as to allow catching all errors, * regardless of whether a handler returns/uses promises * or not. - * - * @param {Router} router object + * @param {!Object} route the object containing the router and path to bind it to + * @param {!Application} app the application object */ -function wrapRouteHandlers(router) { - router.stack.forEach((routerLayer) => { +function wrapRouteHandlers(route, app) { + route.router.stack.forEach((routerLayer) => { + let path = (route.path + routerLayer.route.path.slice(1)) + .replace(/\/:/g, '/--') + .replace(/^\//, '') + .replace(/[/?]+$/, ''); + path = app.metrics.normalizeName(path || 'root'); routerLayer.route.stack.forEach((layer) => { const origHandler = layer.handle; - // eslint-disable-next-line no-param-reassign - layer.handle = function handle(req, res, next) { + // eslint-disable-next-line no-param-reassign,func-names + layer.handle = function (req, res, next) { + const startTime = Date.now(); BBPromise.try(() => origHandler(req, res, next)) - .catch(next); + .catch(next) + .finally(() => { + let statusCode = parseInt(res.statusCode, 10) || 500; + if (statusCode < 100 || statusCode > 599) { + statusCode = 500; + } + const statusClass = `${Math.floor(statusCode / 100)}xx`; + const stat = `${path}.${req.method}.`; + app.metrics.endTiming([ + stat + statusCode, + stat + statusClass, + `${stat}ALL`, + ], startTime); + }); }; }); }); @@ -129,13 +132,12 @@ function wrapRouteHandlers(router) { /** - * 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) { - // Note: the 'next' parameter is required for correct error handling + // Note: the 'next' parameter is required for correct error handling. + // Do not remove it from the callback. // eslint-disable-next-line no-unused-vars app.use((err, req, res, next) => { let errObj; @@ -193,11 +195,8 @@ function setErrorHandler(app) { 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 const respBody = { status: errObj.status, @@ -214,9 +213,8 @@ function setErrorHandler(app) { /** * 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) { const options = { @@ -224,25 +222,25 @@ function createRouter(opts) { }; if (opts && opts.constructor === Object) { - Object.keys(opts).forEach((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' }); } diff --git a/package.json b/package.json index 89e9af0..c5cec72 100644 --- a/package.json +++ b/package.json @@ -5,7 +5,9 @@ "main": "./app.js", "scripts": { "start": "service-runner", - "test": "grunt test", + "test": "PREQ_CONNECT_TIMEOUT=15 mocha --exit", + "lint": "eslint --max-warnings 0 --ext .js .", + "nsp": "nsp check", "docker-start": "service-runner docker-start", "docker-test": "service-runner docker-test", "coverage": "istanbul cover _mocha -- -R spec" @@ -25,23 +27,22 @@ "bugs": "https://github.com/kartotherian/tilerator/issues", "homepage": "https://github.com/kartotherian/tilerator", "dependencies": { - "bluebird": "^3.5.0", - "body-parser": "^1.15.0", - "bunyan": "^1.8.1", + "bluebird": "^3.5.1", + "body-parser": "^1.18.3", + "bunyan": "^1.8.12", "cassandra-uuid": "^0.0.2", - "compression": "^1.6.1", - "domino": "^1.0.24", - "express": "^4.13.4", - "js-yaml": "^3.8.2", - "preq": "^0.5.2", - "service-runner": "^2.2.5", - "heapdump": "*", + "compression": "^1.7.2", + "domino": "^2.0.2", + "express": "^4.16.3", + "http-shutdown": "^1.2.0", + "js-yaml": "^3.12.0", + "preq": "^0.5.6", + "service-runner": "^2.6.3", + "swagger-router": "^0.7.1", + "swagger-ui": "git+https://github.com/wikimedia/swagger-ui#master", "minimist": "0.2.*", "yargs": "^5.0.0", "underscore": "^1.8.3", - "why-is-node-running": "^1.2.2", - "htcp-purge": "^0.1.2", - "jade": "^1.11.0", "kue": "^0.11.0", "kue-ui": "git+https://github.com/nyurik/kue-ui.git", "mapnik": "~3.5.0", @@ -70,24 +71,25 @@ "bunyan-prettystream": "*" }, "devDependencies": { + "ajv": "^6.5.1", + "eslint": "^4.19.1", "eslint-config-airbnb-base": "^12.1.0", "eslint-config-kartotherian": "^0.0.5", - "extend": "^3.0.0", - "grunt": "^1.0.2", - "grunt-contrib-watch": "^1.0.0", - "grunt-eslint": "^20.1.0", - "grunt-mocha-test": "^0.13.3", - "istanbul": "^0.4.3", - "mocha": "^5.0.4", + "eslint-plugin-jsdoc": "^3.7.1", + "eslint-plugin-json": "^1.2.0", + "extend": "^3.0.1", + "istanbul": "^0.4.5", + "mocha": "^5.2.0", "mocha-lcov-reporter": "^1.2.0", - "swagger-router": "^0.4.2", + "mocha-eslint": "^4.1.0", + "nsp": "^3.2.1", "tilelive-file": "~0.0.3", "tilelive-http": "~0.13.0", "wait-as-promised": "^1.0.2" }, "deploy": { - "node": "6.11", "target": "debian", + "node": "6.11.1", "install_opts": [ "--build-from-source=mapnik", "--fallback-to-build=false" diff --git a/routes/empty.js.template b/routes/empty.js.template deleted file mode 100644 index 97cff86..0000000 --- a/routes/empty.js.template +++ /dev/null @@ -1,39 +0,0 @@ -'use strict'; - - -var BBPromise = require('bluebird'); -var preq = require('preq'); -var sUtil = require('../lib/util'); - -// shortcut -var HTTPError = sUtil.HTTPError; - - -/** - * The main router object - */ -var router = sUtil.router(); - -/** - * The main application object reported when this module is require()d - */ -var app; - - -/** ROUTE DECLARATIONS GO HERE **/ - - -module.exports = function(appObj) { - - app = appObj; - - // the returned object mounts the routes on - // /{domain}/vX/mount/path - return { - path: '/mount/path', - api_version: X, // must be a number! - router: router - }; - -}; - diff --git a/routes/root.js b/routes/root.js index bf2008a..1dc0104 100644 --- a/routes/root.js +++ b/routes/root.js @@ -1,4 +1,6 @@ const sUtil = require('../lib/util'); +const swaggerUi = require('../lib/swagger-ui'); + /** * The main router object @@ -10,6 +12,7 @@ const router = sUtil.router(); */ let app; + /** * GET /robots.txt * Instructs robots no indexing should occur on this domain. @@ -24,14 +27,16 @@ router.get('/robots.txt', (req, res) => { /** * 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 (!Object.prototype.hasOwnProperty.call((req.query || {}), 'spec')) { - next(); - } else { +router.get('/', (req, res, next) => { // eslint-disable-line consistent-return + 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(); } }); diff --git a/spec.template.yaml b/spec.template.yaml index e42641a..27ee54d 100644 --- a/spec.template.yaml +++ b/spec.template.yaml @@ -1,6 +1,6 @@ swagger: '2.0' info: - version: 0.2.1 + version: 0.4.0 title: WMF Node.JS Service Template description: A template for creating Node.JS services termsOfService: https://wikimediafoundation.org/wiki/Terms_of_Use @@ -12,6 +12,23 @@ info: url: http://www.apache.org/licenses/LICENSE-2.0 x-default-params: domain: en.wikipedia.org + +parameters: + domain: + in: path + name: domain + required: true + type: string + description: | + Project domain for the requested data. + title: + in: path + name: title + required: true + type: string + description: | + Page title. Use underscores instead of spaces. Example: `Main_Page` + paths: # from routes/root.js /robots.txt: @@ -20,6 +37,13 @@ paths: - Root - Robots description: Gets robots.txt + responses: + 200: + description: Success + default: + description: Error + schema: + $ref: '#/definitions/problem' x-amples: - title: robots.txt check request: {} @@ -35,6 +59,13 @@ paths: description: The root service end-point produces: - application/json + responses: + 200: + description: Success + default: + description: Error + schema: + $ref: '#/definitions/problem' x-amples: - title: root with no query params request: {} @@ -46,6 +77,12 @@ paths: spec: true response: status: 200 + - title: doc from root + request: + query: + doc: true + response: + status: 200 - title: root with wrong query param request: query: @@ -62,6 +99,23 @@ paths: description: Calls the MW API siteinfo action and returns the response produces: - application/json + parameters: + - $ref: '#/parameters/domain' + responses: + 200: + description: Success + 404: + description: Not Found + schema: + $ref: '#/definitions/problem' + 504: + description: Server Error + schema: + $ref: '#/definitions/problem' + default: + description: Error + schema: + $ref: '#/definitions/problem' x-amples: - title: site info for default domain request: {} @@ -104,6 +158,20 @@ paths: produces: - text/html - application/json + parameters: + - $ref: '#/parameters/domain' + - $ref: '#/parameters/title' + responses: + 200: + description: OK + 404: + description: Not Found + schema: + $ref: '#/definitions/problem' + default: + description: Error + schema: + $ref: '#/definitions/problem' x-amples: - title: get the Foobar page from en.wp.org request: @@ -123,6 +191,20 @@ paths: produces: - text/html - application/json + parameters: + - $ref: '#/parameters/domain' + - $ref: '#/parameters/title' + responses: + 200: + description: OK + 404: + description: Not Found + schema: + $ref: '#/definitions/problem' + default: + description: Error + schema: + $ref: '#/definitions/problem' x-amples: - title: get the lead section for Barack Obama request: @@ -140,6 +222,9 @@ paths: description: Gets information about the service produces: - application/json + responses: + 200: + description: OK x-amples: - title: retrieve service info request: {} @@ -160,6 +245,9 @@ paths: description: Gets the name of the service produces: - application/json + responses: + 200: + description: OK x-amples: - title: retrieve service name request: {} @@ -177,6 +265,9 @@ paths: description: Gets the running version of the service produces: - application/json + responses: + 200: + description: OK x-amples: - title: retrieve service version request: {} @@ -192,6 +283,9 @@ paths: - Service information - Service homepage description: Redirects to the home page + responses: + 301: + description: Redirect x-amples: - title: redirect to the home page request: {} @@ -207,6 +301,11 @@ paths: description: Generates an internal error due to a wrong array declaration produces: - application/json + responses: + 500: + description: Internal Error + schema: + $ref: '#/definitions/problem' x-amples: - title: wrong array declaration example request: {} @@ -223,6 +322,11 @@ paths: description: Generates an internal error due to a non-existing file produces: - application/json + responses: + 500: + description: Internal Error + schema: + $ref: '#/definitions/problem' x-amples: - title: non-existing file example request: {} @@ -239,6 +343,11 @@ paths: description: Generates an internal error due to a user-thrown error produces: - application/json + responses: + 500: + description: Internal Error + schema: + $ref: '#/definitions/problem' x-amples: - title: user error example request: {} @@ -255,6 +364,11 @@ paths: description: Generates an access-denied error produces: - application/json + responses: + 403: + description: Access Denied + schema: + $ref: '#/definitions/problem' x-amples: - title: access denied error example request: {} @@ -271,6 +385,11 @@ paths: description: Generates an unauthorised error produces: - application/json + responses: + 401: + description: Unauthorized + schema: + $ref: '#/definitions/problem' x-amples: - title: unauthorised error example request: {} @@ -278,3 +397,22 @@ paths: status: 401 headers: content-type: application/json + +definitions: + # A https://tools.ietf.org/html/draft-nottingham-http-problem + problem: + required: + - type + properties: + status: + type: integer + type: + type: string + title: + type: string + detail: + type: string + method: + type: string + uri: + type: string \ No newline at end of file diff --git a/test/.eslintrc.js b/test/.eslintrc.js deleted file mode 100644 index d3fa9c2..0000000 --- a/test/.eslintrc.js +++ /dev/null @@ -1,7 +0,0 @@ -module.exports = { - extends: '../.eslintrc.js', - rules: { - // It is okay to import devDependencies in tests. - 'import/no-extraneous-dependencies': [2, { devDependencies: true }], - }, -}; diff --git a/test/features/app/app.js b/test/features/app/app.js index e1b9677..4279007 100644 --- a/test/features/app/app.js +++ b/test/features/app/app.js @@ -1,7 +1,5 @@ -/* global describe it before */ +/* global describe, it, before, after */ -// eslint-disable-next-line strict,lines-around-directive -'use strict'; const fs = require('fs'); const preq = require('preq'); @@ -31,8 +29,13 @@ function fileExists(file) { return false; } -describe('express app', function expressApp() { - this.timeout(20000); +if (!server.stopHookAdded) { + server.stopHookAdded = true; + after(() => server.stop()); +} + +describe('express app', function () { // eslint-disable-line func-names + this.timeout(20000); // eslint-disable-line no-invalid-this before(() => server.start()); @@ -43,6 +46,28 @@ describe('express app', function expressApp() { assert.deepEqual(res.headers.disallow, '/'); })); + it('should get static content gzipped', () => preq.get({ + uri: `${server.config.uri}static/admin.html`, + headers: { + 'accept-encoding': 'gzip, deflate', + }, + }).then((res) => { + assert.deepEqual(res.status, 200); + // if there is no content-length, the reponse was gzipped + assert.deepEqual(res.headers['content-length'], undefined, 'Did not expect the content-length header!'); + })); + + it('should get static content uncompressed', () => preq.get({ + uri: `${server.config.uri}static/admin.html`, + headers: { + 'accept-encoding': '', + }, + }).then((res) => { + const contentEncoding = res.headers['content-encoding']; + assert.deepEqual(res.status, 200); + assert.deepEqual(contentEncoding, undefined, 'Did not expect gzipped contents!'); + })); + it('moves a tile from source to destination', () => { // ensure file doesn't exist yet deleteIfExist('test/filestore/6/33/22.png'); diff --git a/test/features/app/spec.js b/test/features/app/spec.js index 3513b32..c40aad3 100644 --- a/test/features/app/spec.js +++ b/test/features/app/spec.js @@ -1,7 +1,5 @@ -/* global describe it before */ +/* global describe, it, before, after */ -// eslint-disable-next-line strict,lines-around-directive -'use strict'; const preq = require('preq'); const assert = require('../../utils/assert.js'); @@ -10,6 +8,11 @@ const { URI } = require('swagger-router'); const yaml = require('js-yaml'); const fs = require('fs'); +if (!server.stopHookAdded) { + server.stopHookAdded = true; + after(() => server.stop()); +} + function staticSpecLoad() { let spec; const myService = server.config.conf.services[server.config.conf.services.length - 1].conf; @@ -85,11 +88,10 @@ function constructTests(paths, defParams) { Object.keys(paths).forEach((pathStr) => { Object.keys(paths[pathStr]).forEach((method) => { const p = paths[pathStr][method]; - const uri = new URI(pathStr, {}, true); - - if (Object.prototype.hasOwnProperty.call(p, 'x-monitor') && !p['x-monitor']) { + if ({}.hasOwnProperty.call(p, 'x-monitor') && !p['x-monitor']) { return; } + const uri = new URI(pathStr, {}, true); if (!p['x-amples']) { ret.push(constructTestCase( pathStr, @@ -131,7 +133,10 @@ function cmp(result, expected, errMsg) { if (expected.constructor === Object) { Object.keys(expected).forEach((key) => { const val = expected[key]; - assert.deepEqual(Object.prototype.hasOwnProperty.call(result, key), true, `Body field ${key} not found in response!`); + assert.deepEqual( + {}.hasOwnProperty.call(result, key), true, + `Body field ${key} not found in response!` + ); cmp(result[key], val, `${key} body field mismatch!`); }); return true; @@ -181,7 +186,10 @@ function validateTestResponse(testCase, res) { // check the headers Object.keys(expRes.headers).forEach((key) => { const val = expRes.headers[key]; - assert.deepEqual(Object.prototype.hasOwnProperty.call(res.headers, key), true, `Header ${key} not found in response!`); + 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 @@ -199,7 +207,7 @@ function validateTestResponse(testCase, res) { } // 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}`); + throw new Error(`Expected body type ${expRes.body.constructor} but got ${res.body.constructor}`); } // compare the bodies @@ -240,13 +248,11 @@ describe('Swagger spec', function swaggerSpecTest() { // now check each path Object.keys(spec.paths).forEach((pathStr) => { const path = spec.paths[pathStr]; - assert.deepEqual(!!pathStr, true, 'A path cannot have a length of zero!'); assert.deepEqual(!!Object.keys(path), true, `No methods defined for path: ${pathStr}`); - Object.keys(path).forEach((method) => { const mSpec = path[method]; - if (Object.prototype.hasOwnProperty.call(mSpec, 'x-monitor') && !mSpec['x-monitor']) { + if ({}.hasOwnProperty.call(mSpec, 'x-monitor') && !mSpec['x-monitor']) { return; } validateExamples(pathStr, defParams, mSpec['x-amples']); diff --git a/test/features/info/info.js b/test/features/info/info.js index 4c8cfd2..203a059 100644 --- a/test/features/info/info.js +++ b/test/features/info/info.js @@ -1,12 +1,14 @@ -/* global describe it before */ +/* global describe, it, before, after */ -// eslint-disable-next-line strict,lines-around-directive -'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 serviceInfo() { this.timeout(20000); diff --git a/test/index.js b/test/index.js new file mode 100644 index 0000000..70e3694 --- /dev/null +++ b/test/index.js @@ -0,0 +1,7 @@ +// Run eslint as part of normal testing +require('mocha-eslint')([ + 'lib', + 'routes', +], { + timeout: 10000, +}); diff --git a/test/utils/assert.js b/test/utils/assert.js index 1f715ff..43a82ec 100644 --- a/test/utils/assert.js +++ b/test/utils/assert.js @@ -1,16 +1,14 @@ +/* eslint-disable no-console */ + + 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); - } + assert.deepEqual(result, expected, message); } catch (e) { - // eslint-disable-next-line no-console console.log(`Expected:\n${JSON.stringify(expected, null, 2)}`); - // eslint-disable-next-line no-console console.log(`Result:\n${JSON.stringify(result, null, 2)}`); throw e; } @@ -30,35 +28,30 @@ function status(res, expected) { /** * Asserts whether content type was as expected */ -function contentType(res, expected) { +function contentType(res, expectedRegexString) { const actual = res.headers['content-type']; - deepEqual( - actual, expected, - `Expected content-type to be ${expected}, but was ${actual}` + assert.ok( + RegExp(expectedRegexString).test(actual), + `Expected content-type to match ${expectedRegexString}, but was ${actual}` ); } function isDeepEqual(result, expected, message) { try { - if (typeof expected === 'string') { - assert.ok(result === expected || (new RegExp(expected).test(result)), message); - } else { - assert.deepEqual(result, expected, message); - } + assert.deepEqual(result, expected, message); return true; } catch (e) { return false; } } + function notDeepEqual(result, expected, message) { try { assert.notDeepEqual(result, expected, message); } catch (e) { - // eslint-disable-next-line no-console console.log(`Not expected:\n${JSON.stringify(expected, null, 2)}`); - // eslint-disable-next-line no-console console.log(`Result:\n${JSON.stringify(result, null, 2)}`); throw e; } diff --git a/test/utils/logStream.js b/test/utils/logStream.js index 7ca6131..25dd263 100644 --- a/test/utils/logStream.js +++ b/test/utils/logStream.js @@ -1,3 +1,6 @@ +/* eslint-disable no-console */ + + const bunyan = require('bunyan'); function logStream(logStdout) { @@ -18,7 +21,6 @@ function logStream(logStdout) { } } } catch (e) { - // eslint-disable-next-line no-console console.error('something went wrong trying to parrot a log entry', e, chunk); } @@ -35,29 +37,31 @@ function logStream(logStdout) { function slice() { const begin = log.length; - let end2 = null; + let end = null; // eslint-disable-line no-shadow function halt() { - if (end2 === null) { - end2 = log.length; + if (end === null) { + end = log.length; } } - function get2() { + function get() { // eslint-disable-line no-shadow return log.slice(begin, end); } + /* Disable eslint object-shorthand until Node 4 support is dropped */ + /* eslint-disable object-shorthand */ return { - halt, - get: get2, + halt: halt, + get: get, }; } return { - write, - end, - slice, - get, + write: write, + end: end, + slice: slice, + get: get, }; } diff --git a/test/utils/server.js b/test/utils/server.js index a4f5a97..3a8abea 100644 --- a/test/utils/server.js +++ b/test/utils/server.js @@ -1,10 +1,12 @@ 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 let config = { conf: yaml.safeLoad(fs.readFileSync(`${__dirname}/../../config.test.yaml`)), @@ -18,33 +20,43 @@ config.service = myService; // no forking, run just one process when testing config.conf.num_workers = 0; // have a separate, in-memory logger only +config.conf.logging = { + name: 'test-log', + level: 'trace', + stream: logStream(), +}; // make a deep copy of it for later reference const origConfig = extend(true, {}, config); -let stop = function stop() {}; +module.exports.stop = () => BBPromise.resolve(); let options = null; const runner = new ServiceRunner(); + function start(_options) { - const normalizedOptions = _options || {}; + _options = _options || {}; // eslint-disable-line no-param-reassign - if (!assert.isDeepEqual(options, normalizedOptions)) { - stop(); - options = normalizedOptions; - // set up the config - config = extend(true, {}, origConfig); - extend(true, config.conf.services[myServiceIdx].conf, options); - return runner.start(config.conf) - .then((servers) => { - const server = servers[0]; - // eslint-disable-next-line no-shadow - stop = function stop() { - server.close(); - // eslint-disable-next-line no-func-assign,no-shadow - stop = function stop() {}; - }; - return true; - }); + if (!assert.isDeepEqual(options, _options)) { + 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((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 = () => BBPromise.resolve(); + }); + }; + return true; + }); + }); } return BBPromise.resolve(); }