From aa1b046cf2c9def411cf72d2195dbe9684886ef8 Mon Sep 17 00:00:00 2001 From: Matteo Pagliazzi Date: Tue, 17 Nov 2015 16:48:50 +0100 Subject: [PATCH 1/9] add example apidoc comments, add notFound middleware --- .gitignore | 1 + .../v3/unit/middlewares/errorHandler.test.js | 2 +- test/api/v3/unit/middlewares/notFound.test.js | 32 +++++++++++++++++++ website/src/controllers/api-v3/example.js | 21 +++++++++++- website/src/libs/api-v3/errors.js | 19 +++++++++++ .../src/middlewares/api-v3/errorHandler.js | 20 ++++++------ website/src/middlewares/api-v3/index.js | 2 ++ website/src/middlewares/api-v3/notFound.js | 7 ++++ website/src/server.js | 19 +++++------ 9 files changed, 102 insertions(+), 21 deletions(-) create mode 100644 test/api/v3/unit/middlewares/notFound.test.js create mode 100644 website/src/middlewares/api-v3/notFound.js diff --git a/.gitignore b/.gitignore index 911f7f45e9..78a2bf4294 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,7 @@ .DS_Store website/public/gen website/public/common +website/public/apidoc node_modules *.swp .idea* diff --git a/test/api/v3/unit/middlewares/errorHandler.test.js b/test/api/v3/unit/middlewares/errorHandler.test.js index 912dd75d1c..783e0aff21 100644 --- a/test/api/v3/unit/middlewares/errorHandler.test.js +++ b/test/api/v3/unit/middlewares/errorHandler.test.js @@ -88,7 +88,7 @@ describe('errorHandler', () => { errorHandler(error, req, res, next); expect(logger.error).to.be.calledOnce; - expect(logger.error).to.be.calledWith(error.stack, { + expect(logger.error).to.be.calledWithExactly(error.stack, { originalUrl: req.originalUrl, headers: req.headers, body: req.body, diff --git a/test/api/v3/unit/middlewares/notFound.test.js b/test/api/v3/unit/middlewares/notFound.test.js new file mode 100644 index 0000000000..55064dbe0c --- /dev/null +++ b/test/api/v3/unit/middlewares/notFound.test.js @@ -0,0 +1,32 @@ +import { + generateRes, + generateReq, + generateNext, +} from '../../../../helpers/api-unit.helper'; + +import notFoundHandler from '../../../../../website/src/middlewares/api-v3/notFound'; + +import { NotFound } from '../../../../../website/src/libs/api-v3/errors'; + +describe('notFoundHandler', () => { + let res, req, next; + + beforeEach(() => { + res = generateRes(); + req = generateReq(); + next = generateNext(); + + sandbox.stub(logger, 'error'); + }); + + it('sends NotFound error if the resource isn\'t found', () => { + expect(res.status).to.be.calledOnce; + expect(res.json).to.be.calledOnce; + + expect(res.status).to.be.calledWith(404); + expect(res.json).to.be.calledWith({ + error: 'NotFound', + message: 'Not found.', + }); + }); +}); diff --git a/website/src/controllers/api-v3/example.js b/website/src/controllers/api-v3/example.js index f5e9f24b71..7ead22d794 100644 --- a/website/src/controllers/api-v3/example.js +++ b/website/src/controllers/api-v3/example.js @@ -1,6 +1,25 @@ // An example file to show how a controller should be structured let api = {}; +/** + * @api {get} /example/:id Request Example information + * @apiName GetExample + * @apiGroup Example + * + * @apiParam {Number} id Examples unique ID. + * + * @apiSuccess {String} firstname Firstname of the Example. + * @apiSuccess {String} lastname Lastname of the Example. + * + * @apiSuccessExample Success-Response: + * HTTP/1.1 200 OK + * { + * "firstname": "John", + * "lastname": "Doe" + * } + * + * @apiUse NotFound + */ api.exampleRoute = { method: 'GET', url: '/example/:param', @@ -12,4 +31,4 @@ api.exampleRoute = { }, }; -export default api; \ No newline at end of file +export default api; diff --git a/website/src/libs/api-v3/errors.js b/website/src/libs/api-v3/errors.js index 1439669d20..49e3a12c45 100644 --- a/website/src/libs/api-v3/errors.js +++ b/website/src/libs/api-v3/errors.js @@ -30,6 +30,25 @@ export class BadRequest extends CustomError { } } +/** + * @apiDefine NotFound + * @apiError NotFound The requested resource was not found. + * + * @apiErrorExample Error-Response: + * HTTP/1.1 404 Not Found + * { + * "error": "NotFound" + * } + */ +export class NotFound extends CustomError { + constructor (customMessage) { + super(); + this.name = this.constructor.name; + this.httpCode = 401; + this.message = customMessage || 'Not found.'; + } +} + // InternalError error with a 500 http error code // used when an unexpected, internal server error is thrown export class InternalServerError extends CustomError { diff --git a/website/src/middlewares/api-v3/errorHandler.js b/website/src/middlewares/api-v3/errorHandler.js index 5bf90f6969..5982284895 100644 --- a/website/src/middlewares/api-v3/errorHandler.js +++ b/website/src/middlewares/api-v3/errorHandler.js @@ -10,16 +10,6 @@ import { export default function errorHandler (err, req, res, next) { if (!err) return next(); - // Log the original error with some metadata - let stack = err.stack || err.message || err; - - logger.error(stack, { - originalUrl: req.originalUrl, - headers: req.headers, - body: req.body, - fullError: err, - }); - // In case of a CustomError class, use it's data // Otherwise try to identify the type of error (mongoose validation, mongodb unique, ...) // If we can't identify it, respond with a generic 500 error @@ -48,6 +38,16 @@ export default function errorHandler (err, req, res, next) { responseErr = new InternalServerError(); } + // Log the original error with some metadata + let stack = err.stack || err.message || err; + + logger.error(stack, { + originalUrl: req.originalUrl, + headers: req.headers, + body: req.body, + fullError: err, + }); + // TODO unless status >= 500 return data attached to errors return res .status(responseErr.httpCode) diff --git a/website/src/middlewares/api-v3/index.js b/website/src/middlewares/api-v3/index.js index 39847e67bf..0041e2dd54 100644 --- a/website/src/middlewares/api-v3/index.js +++ b/website/src/middlewares/api-v3/index.js @@ -4,6 +4,7 @@ import analytics from './analytics'; import errorHandler from './errorHandler'; import bodyParser from 'body-parser'; import routes from '../../libs/api-v3/setupRoutes'; +import notFoundHandler from './notFound'; export default function attachMiddlewares (app) { // Parse query parameters and json bodies @@ -15,6 +16,7 @@ export default function attachMiddlewares (app) { app.use(analytics); app.use(routes); + app.use(notFoundHandler); // Error handler middleware, define as the last one app.use(errorHandler); diff --git a/website/src/middlewares/api-v3/notFound.js b/website/src/middlewares/api-v3/notFound.js new file mode 100644 index 0000000000..733a247d1d --- /dev/null +++ b/website/src/middlewares/api-v3/notFound.js @@ -0,0 +1,7 @@ +import { + NotFound, +} from '../../libs/api-v3/errors'; + +export default function (req, res, next) { + next(new NotFound()); +} diff --git a/website/src/server.js b/website/src/server.js index 1142f27539..de899ab62d 100644 --- a/website/src/server.js +++ b/website/src/server.js @@ -4,7 +4,7 @@ import nconf from 'nconf'; import logger from './libs/api-v3/logger'; import express from 'express'; import http from 'http'; -// import path from 'path'; +import path from 'path'; // let swagger = require('swagger-node-express'); import autoinc from 'mongoose-id-autoinc'; import passport from 'passport'; @@ -73,7 +73,7 @@ passport.use(new FacebookStrategy({ }, (accessToken, refreshToken, profile, done) => done(null, profile))); // ------------ Server Configuration ------------ -// let publicDir = path.join(__dirname, '/../public'); +let publicDir = path.join(__dirname, '/../public'); app.set('port', nconf.get('PORT')); @@ -144,17 +144,18 @@ oldApp.use('/api/v1', require('./routes/api-v1')); oldApp.use('/export', require('./routes/dataexport')); require('./routes/api-v2/swagger')(swagger, v2); -var maxAge = IS_PROD ? 31536000000 : 0; // Cache emojis without copying them to build, they are too many -oldApp.use(express['static'](path.join(__dirname, "/../build"), { maxAge: maxAge })); -oldApp.use('/common/dist', express['static'](publicDir + "/../../common/dist", { maxAge: maxAge })); -oldApp.use('/common/audio', express['static'](publicDir + "/../../common/audio", { maxAge: maxAge })); -oldApp.use('/common/script/public', express['static'](publicDir + "/../../common/script/public", { maxAge: maxAge })); -oldApp.use('/common/img', express['static'](publicDir + "/../../common/img", { maxAge: maxAge })); -oldApp.use(express['static'](publicDir)); oldApp.use(require('./middlewares/api-v2/errorHandler')); */ +let maxAge = IS_PROD ? 31536000000 : 0; + +oldApp.use(express.static(path.join(__dirname, '/../build'), { maxAge })); +oldApp.use('/common/dist', express.static(`${publicDir}/../../common/dist`, { maxAge })); +oldApp.use('/common/audio', express.static(`${publicDir}/../../common/audio`, { maxAge })); +oldApp.use('/common/script/public', express.static(`${publicDir}/../../common/script/public`, { maxAge })); +oldApp.use('/common/img', express.static(`${publicDir}/../../common/img`, { maxAge })); +oldApp.use(express.static(publicDir)); server.on('request', app); server.listen(app.get('port'), () => { From 9905deec060631bc7d1d797a8ab5b59f756d3b28 Mon Sep 17 00:00:00 2001 From: Matteo Pagliazzi Date: Tue, 17 Nov 2015 17:02:53 +0100 Subject: [PATCH 2/9] move static handler to separate middleware --- website/src/middlewares/api-v3/static.js | 18 ++++++++++++++++++ website/src/server.js | 10 +++++++--- 2 files changed, 25 insertions(+), 3 deletions(-) create mode 100644 website/src/middlewares/api-v3/static.js diff --git a/website/src/middlewares/api-v3/static.js b/website/src/middlewares/api-v3/static.js new file mode 100644 index 0000000000..2fc141ac1b --- /dev/null +++ b/website/src/middlewares/api-v3/static.js @@ -0,0 +1,18 @@ +import express from 'express'; +import nconf from 'nconf'; +import path from 'path'; + +const IS_PROD = nconf.get('IS_PROD'); +const MAX_AGE = IS_PROD ? 31536000000 : 0; +const PUBLIC_DIR = path.join(__dirname, '/../../../public'); +const BUILD_DIR = path.join(__dirname, '/../../../build'); + +export default function staticMiddleware (expressApp) { + // TODO move all static files to a single location (one for public and one for build) + expressApp.use(express.static(BUILD_DIR, { maxAge: MAX_AGE })); + expressApp.use('/common/dist', express.static(`${PUBLIC_DIR}/../../common/dist`, { maxAge: MAX_AGE })); + expressApp.use('/common/audio', express.static(`${PUBLIC_DIR}/../../common/audio`, { maxAge: MAX_AGE })); + expressApp.use('/common/script/public', express.static(`${PUBLIC_DIR}/../../common/script/public`, { maxAge: MAX_AGE })); + expressApp.use('/common/img', express.static(`${PUBLIC_DIR}/../../common/img`, { maxAge: MAX_AGE })); + expressApp.use(express.static(PUBLIC_DIR)); +}; diff --git a/website/src/server.js b/website/src/server.js index de899ab62d..21744c0813 100644 --- a/website/src/server.js +++ b/website/src/server.js @@ -4,7 +4,7 @@ import nconf from 'nconf'; import logger from './libs/api-v3/logger'; import express from 'express'; import http from 'http'; -import path from 'path'; +// import path from 'path'; // let swagger = require('swagger-node-express'); import autoinc from 'mongoose-id-autoinc'; import passport from 'passport'; @@ -14,6 +14,7 @@ import mongoose from 'mongoose'; import Q from 'q'; import domainMiddleware from './middlewares/api-v3/domain'; import attachMiddlewares from './middlewares/api-v3/index'; +import staticMiddleware from './middlewares/api-v3/static'; // Setup translations // let i18n = require('./libs/api-v2/i18n'); @@ -73,7 +74,7 @@ passport.use(new FacebookStrategy({ }, (accessToken, refreshToken, profile, done) => done(null, profile))); // ------------ Server Configuration ------------ -let publicDir = path.join(__dirname, '/../public'); +// let publicDir = path.join(__dirname, '/../public'); app.set('port', nconf.get('PORT')); @@ -147,7 +148,7 @@ require('./routes/api-v2/swagger')(swagger, v2); // Cache emojis without copying them to build, they are too many oldApp.use(require('./middlewares/api-v2/errorHandler')); -*/ +* let maxAge = IS_PROD ? 31536000000 : 0; oldApp.use(express.static(path.join(__dirname, '/../build'), { maxAge })); @@ -156,6 +157,9 @@ oldApp.use('/common/audio', express.static(`${publicDir}/../../common/audio`, { oldApp.use('/common/script/public', express.static(`${publicDir}/../../common/script/public`, { maxAge })); oldApp.use('/common/img', express.static(`${publicDir}/../../common/img`, { maxAge })); oldApp.use(express.static(publicDir)); +*/ + +staticMiddleware(app); server.on('request', app); server.listen(app.get('port'), () => { From 846800ccc9dd052fc7c9bf3f2f3ce3ed26163db7 Mon Sep 17 00:00:00 2001 From: Matteo Pagliazzi Date: Tue, 17 Nov 2015 17:49:59 +0100 Subject: [PATCH 3/9] fix linting --- website/src/middlewares/api-v3/static.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/src/middlewares/api-v3/static.js b/website/src/middlewares/api-v3/static.js index 2fc141ac1b..f944e1e301 100644 --- a/website/src/middlewares/api-v3/static.js +++ b/website/src/middlewares/api-v3/static.js @@ -15,4 +15,4 @@ export default function staticMiddleware (expressApp) { expressApp.use('/common/script/public', express.static(`${PUBLIC_DIR}/../../common/script/public`, { maxAge: MAX_AGE })); expressApp.use('/common/img', express.static(`${PUBLIC_DIR}/../../common/img`, { maxAge: MAX_AGE })); expressApp.use(express.static(PUBLIC_DIR)); -}; +} From 2a51117d216b4796c5e65a470a1fe168a6bfdb9f Mon Sep 17 00:00:00 2001 From: Matteo Pagliazzi Date: Tue, 17 Nov 2015 19:22:47 +0100 Subject: [PATCH 4/9] comment errors according to apidoc, add new tests and fix existing ones --- test/api/v3/unit/libs/errors.test.js | 31 +++++++++++- .../v3/unit/middlewares/errorHandler.test.js | 4 +- test/api/v3/unit/middlewares/notFound.test.js | 4 +- website/src/libs/api-v3/errors.js | 49 ++++++++++++++----- .../src/middlewares/api-v3/errorHandler.js | 20 ++++---- 5 files changed, 80 insertions(+), 28 deletions(-) diff --git a/test/api/v3/unit/libs/errors.test.js b/test/api/v3/unit/libs/errors.test.js index e859d2bb02..15110a06ee 100644 --- a/test/api/v3/unit/libs/errors.test.js +++ b/test/api/v3/unit/libs/errors.test.js @@ -3,6 +3,7 @@ import { NotAuthorized, BadRequest, InternalServerError, + NotFound, } from '../../../../../website/src/libs/api-v3/errors'; describe('Custom Errors', () => { @@ -21,7 +22,7 @@ describe('Custom Errors', () => { expect(notAuthorizedError).to.be.an.instanceOf(CustomError); }); - it('it returns an http code of 400', () => { + it('it returns an http code of 401', () => { let notAuthorizedError = new NotAuthorized(); expect(notAuthorizedError.httpCode).to.eql(401); @@ -40,6 +41,32 @@ describe('Custom Errors', () => { }); }); + describe('NotFound', () => { + it('is an instance of CustomError', () => { + let notAuthorizedError = new NotFound(); + + expect(notAuthorizedError).to.be.an.instanceOf(CustomError); + }); + + it('it returns an http code of 404', () => { + let notAuthorizedError = new NotFound(); + + expect(notAuthorizedError.httpCode).to.eql(404); + }); + + it('returns a default message', () => { + let notAuthorizedError = new NotFound(); + + expect(notAuthorizedError.message).to.eql('Not found.'); + }); + + it('allows a custom message', () => { + let notAuthorizedError = new NotFound('Custom Error Message'); + + expect(notAuthorizedError.message).to.eql('Custom Error Message'); + }); + }); + describe('BadRequest', () => { it('is an instance of CustomError', () => { let badRequestError = new BadRequest(); @@ -82,7 +109,7 @@ describe('Custom Errors', () => { it('returns a default message', () => { let internalServerError = new InternalServerError(); - expect(internalServerError.message).to.eql('Internal server error.'); + expect(internalServerError.message).to.eql('An unexpected error occurred.'); }); it('allows a custom message', () => { diff --git a/test/api/v3/unit/middlewares/errorHandler.test.js b/test/api/v3/unit/middlewares/errorHandler.test.js index 783e0aff21..afc506e976 100644 --- a/test/api/v3/unit/middlewares/errorHandler.test.js +++ b/test/api/v3/unit/middlewares/errorHandler.test.js @@ -31,7 +31,7 @@ describe('errorHandler', () => { expect(res.status).to.be.calledWith(500); expect(res.json).to.be.calledWith({ error: 'InternalServerError', - message: 'Internal server error.', + message: 'An unexpected error occurred.', }); }); @@ -63,7 +63,7 @@ describe('errorHandler', () => { expect(res.status).to.be.calledWith(500); expect(res.json).to.be.calledWith({ error: 'InternalServerError', - message: 'Internal server error.', + message: 'An unexpected error occurred.', }); }); diff --git a/test/api/v3/unit/middlewares/notFound.test.js b/test/api/v3/unit/middlewares/notFound.test.js index 55064dbe0c..5f9922d38c 100644 --- a/test/api/v3/unit/middlewares/notFound.test.js +++ b/test/api/v3/unit/middlewares/notFound.test.js @@ -15,11 +15,9 @@ describe('notFoundHandler', () => { res = generateRes(); req = generateReq(); next = generateNext(); - - sandbox.stub(logger, 'error'); }); - it('sends NotFound error if the resource isn\'t found', () => { + xit('sends NotFound error if the resource isn\'t found', () => { expect(res.status).to.be.calledOnce; expect(res.json).to.be.calledOnce; diff --git a/website/src/libs/api-v3/errors.js b/website/src/libs/api-v3/errors.js index 49e3a12c45..1e90c0158e 100644 --- a/website/src/libs/api-v3/errors.js +++ b/website/src/libs/api-v3/errors.js @@ -7,8 +7,17 @@ export class CustomError extends Error { } } -// NotAuthorized error with a 401 http error code -// used when a request is not authorized +/** + * @apiDefine NotFound + * @apiError NotFound The client is not authorized to make this request. + * + * @apiErrorExample Error-Response: + * HTTP/1.1 401 Unauthorized + * { + * "error": "NotAuthorized", + * "message": "Not authorized." + * } + */ export class NotAuthorized extends CustomError { constructor (customMessage) { super(); @@ -18,10 +27,18 @@ export class NotAuthorized extends CustomError { } } -// BadRequest error with a 400 http error code -// used for requests not formatted correctly -// TODO use for validation errors too? -export class BadRequest extends CustomError { +/** + * @apiDefine BadRequest + * @apiError BadRequest The request wasn't formatted correctly. + * + * @apiErrorExample Error-Response: + * HTTP/1.1 400 Bad Request + * { + * "error": "BadRequest", + * "message": "Bad request." + * } + */ + export class BadRequest extends CustomError { constructor (customMessage) { super(); this.name = this.constructor.name; @@ -37,25 +54,35 @@ export class BadRequest extends CustomError { * @apiErrorExample Error-Response: * HTTP/1.1 404 Not Found * { - * "error": "NotFound" + * "error": "NotFound", + * "message": "Not found." * } */ export class NotFound extends CustomError { constructor (customMessage) { super(); this.name = this.constructor.name; - this.httpCode = 401; + this.httpCode = 404; this.message = customMessage || 'Not found.'; } } -// InternalError error with a 500 http error code -// used when an unexpected, internal server error is thrown +/** + * @apiDefine InternalServerError + * @apiError InternalServerError An unexpected error occurred. + * + * @apiErrorExample Error-Response: + * HTTP/1.1 500 Internal Server Error + * { + * "error": "InternalServerError", + * "message": "An unexpected error occurred." + * } + */ export class InternalServerError extends CustomError { constructor (customMessage) { super(); this.name = this.constructor.name; this.httpCode = 500; - this.message = customMessage || 'Internal server error.'; + this.message = customMessage || 'An unexpected error occurred.'; } } diff --git a/website/src/middlewares/api-v3/errorHandler.js b/website/src/middlewares/api-v3/errorHandler.js index 5982284895..5bf90f6969 100644 --- a/website/src/middlewares/api-v3/errorHandler.js +++ b/website/src/middlewares/api-v3/errorHandler.js @@ -10,6 +10,16 @@ import { export default function errorHandler (err, req, res, next) { if (!err) return next(); + // Log the original error with some metadata + let stack = err.stack || err.message || err; + + logger.error(stack, { + originalUrl: req.originalUrl, + headers: req.headers, + body: req.body, + fullError: err, + }); + // In case of a CustomError class, use it's data // Otherwise try to identify the type of error (mongoose validation, mongodb unique, ...) // If we can't identify it, respond with a generic 500 error @@ -38,16 +48,6 @@ export default function errorHandler (err, req, res, next) { responseErr = new InternalServerError(); } - // Log the original error with some metadata - let stack = err.stack || err.message || err; - - logger.error(stack, { - originalUrl: req.originalUrl, - headers: req.headers, - body: req.body, - fullError: err, - }); - // TODO unless status >= 500 return data attached to errors return res .status(responseErr.httpCode) From 18503e31c3fe7f78f4fb88ccf12b51731e9ab8ef Mon Sep 17 00:00:00 2001 From: Matteo Pagliazzi Date: Tue, 17 Nov 2015 19:31:51 +0100 Subject: [PATCH 5/9] fix linting --- website/src/libs/api-v3/errors.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/src/libs/api-v3/errors.js b/website/src/libs/api-v3/errors.js index 1e90c0158e..89e2bd4fd1 100644 --- a/website/src/libs/api-v3/errors.js +++ b/website/src/libs/api-v3/errors.js @@ -38,7 +38,7 @@ export class NotAuthorized extends CustomError { * "message": "Bad request." * } */ - export class BadRequest extends CustomError { +export class BadRequest extends CustomError { constructor (customMessage) { super(); this.name = this.constructor.name; From 6fb4fdfd8acfbc630fb02f9bb4f439d283c2f154 Mon Sep 17 00:00:00 2001 From: Matteo Pagliazzi Date: Wed, 18 Nov 2015 11:50:55 +0100 Subject: [PATCH 6/9] create apidoc task in grunt, update package.json version --- package.json | 11 ++++++++++- tasks/gulp-apidoc.js | 22 ++++++++++++++++++++++ website/src/controllers/api-v3/example.js | 2 +- 3 files changed, 33 insertions(+), 2 deletions(-) create mode 100644 tasks/gulp-apidoc.js diff --git a/package.json b/package.json index f13295e9e6..d973a03c5c 100644 --- a/package.json +++ b/package.json @@ -1,12 +1,13 @@ { "name": "habitrpg", "description": "A habit tracker app which treats your goals like a Role Playing Game.", - "version": "0.0.0-152", + "version": "3.0.0-alpha", "main": "./website/src/index.js", "dependencies": { "accepts": "^1.3.0", "amazon-payments": "0.0.4", "amplitude": "^2.0.3", + "apidoc": "^0.13.1", "async": "^1.5.0", "aws-sdk": "^2.0.25", "babel-core": "^5.8.34", @@ -117,6 +118,7 @@ "mongodb": "^2.0.46", "mongoskin": "~0.6.1", "nock": "^2.17.0", + "phantomjs": "^1.9.18", "protractor": "~2.5.1", "rewire": "^2.3.3", "rimraf": "^2.4.3", @@ -128,5 +130,12 @@ "uuid": "^2.0.1", "vinyl-source-stream": "^1.0.0", "vinyl-transform": "^1.0.0" + }, + "apidoc": { + "name": "habitica", + "title": "Habitica", + "version": "3.0.0", + "url": "https://habitica.com/api/v3", + "sampleUrl": "https://habitica.com/api/v3" } } diff --git a/tasks/gulp-apidoc.js b/tasks/gulp-apidoc.js new file mode 100644 index 0000000000..cb2777c254 --- /dev/null +++ b/tasks/gulp-apidoc.js @@ -0,0 +1,22 @@ +import gulp from 'gulp'; +import clean from 'rimraf'; +import apidoc from 'apidoc'; + +const APIDOC_DEST_PATH = './website/public/apidoc'; +const APIDOC_SRC_PATH = './website/src'; +gulp.task('apidoc:clean', (done) => { + clean(APIDOC_DEST_PATH, done); +}); + +gulp.task('apidoc', ['apidoc:clean'], (done) => { + let result = apidoc.createDoc({ + src: APIDOC_SRC_PATH, + dest: APIDOC_DEST_PATH, + }); + + if (result === false) { + done(new Error('There was a problem generating apiDoc documentation.')) + } else { + done(); + } +}); diff --git a/website/src/controllers/api-v3/example.js b/website/src/controllers/api-v3/example.js index 7ead22d794..578ce21655 100644 --- a/website/src/controllers/api-v3/example.js +++ b/website/src/controllers/api-v3/example.js @@ -1,8 +1,8 @@ -// An example file to show how a controller should be structured let api = {}; /** * @api {get} /example/:id Request Example information + * @apiVersion 3.0.0 * @apiName GetExample * @apiGroup Example * From b5adf9f19bc3e76f7d0bfca7b3e27b6bca417d1f Mon Sep 17 00:00:00 2001 From: Matteo Pagliazzi Date: Wed, 18 Nov 2015 11:52:13 +0100 Subject: [PATCH 7/9] add apidoc to production build step --- tasks/gulp-build.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tasks/gulp-build.js b/tasks/gulp-build.js index 599d577b10..e7e683ebb9 100644 --- a/tasks/gulp-build.js +++ b/tasks/gulp-build.js @@ -17,6 +17,6 @@ gulp.task('build:dev:watch', ['build:dev'], () => { gulp.watch(['website/public/**/*.styl', 'common/script/*']); }); -gulp.task('build:prod', ['browserify', 'prepare:staticNewStuff'], (done) => { +gulp.task('build:prod', ['browserify', 'prepare:staticNewStuff', 'apidoc'], (done) => { gulp.start('grunt-build:prod', done); }); From 65a8d2e255eadcd92e7d711706483e761e2dd6c1 Mon Sep 17 00:00:00 2001 From: Matteo Pagliazzi Date: Wed, 18 Nov 2015 12:36:13 +0100 Subject: [PATCH 8/9] misc fixes and improvements, starts adding route tests --- test/api/v3/unit/middlewares/notFound.test.js | 37 +++++-------------- website/src/controllers/api-v3/example.js | 4 +- website/src/index.js | 2 - website/src/libs/api-v3/logger.js | 1 + website/src/libs/api-v3/setupRoutes.js | 3 +- website/src/middlewares/api-v3/index.js | 10 ++++- website/src/server.js | 2 +- 7 files changed, 24 insertions(+), 35 deletions(-) diff --git a/test/api/v3/unit/middlewares/notFound.test.js b/test/api/v3/unit/middlewares/notFound.test.js index 5f9922d38c..f2f6082acf 100644 --- a/test/api/v3/unit/middlewares/notFound.test.js +++ b/test/api/v3/unit/middlewares/notFound.test.js @@ -1,30 +1,13 @@ -import { - generateRes, - generateReq, - generateNext, -} from '../../../../helpers/api-unit.helper'; +import { requester } from '../../../../helpers/api-integration.helper'; -import notFoundHandler from '../../../../../website/src/middlewares/api-v3/notFound'; +describe('notFound Middleware', () => { + it('returns a 404 error when the resource is not found', () => { + let request = requester().get('/api/v3/dummy-url'); -import { NotFound } from '../../../../../website/src/libs/api-v3/errors'; - -describe('notFoundHandler', () => { - let res, req, next; - - beforeEach(() => { - res = generateRes(); - req = generateReq(); - next = generateNext(); - }); - - xit('sends NotFound error if the resource isn\'t found', () => { - expect(res.status).to.be.calledOnce; - expect(res.json).to.be.calledOnce; - - expect(res.status).to.be.calledWith(404); - expect(res.json).to.be.calledWith({ - error: 'NotFound', - message: 'Not found.', - }); - }); + return expect(request) + .to.eventually.be.rejected.and.eql({ + error: "NotFound", + message: "Not found.", + }); + }); }); diff --git a/website/src/controllers/api-v3/example.js b/website/src/controllers/api-v3/example.js index 578ce21655..6ad474a5f2 100644 --- a/website/src/controllers/api-v3/example.js +++ b/website/src/controllers/api-v3/example.js @@ -22,11 +22,11 @@ let api = {}; */ api.exampleRoute = { method: 'GET', - url: '/example/:param', + url: '/example/:id', middlewares: [], handler (req, res) { res.status(200).send({ - status: 'ok', + status: req.params.id, }); }, }; diff --git a/website/src/index.js b/website/src/index.js index 2440dde36f..a562dfa9c2 100644 --- a/website/src/index.js +++ b/website/src/index.js @@ -14,8 +14,6 @@ var IS_PROD = nconf.get('IS_PROD'); var IS_DEV = nconf.get('IS_DEV'); var cores = Number(nconf.get('WEB_CONCURRENCY')) || 0; -if (IS_DEV) Error.stackTraceLimit = Infinity; - // Setup the cluster module if (cores !== 0 && cluster.isMaster && (IS_DEV || IS_PROD)) { // Fork workers. If config.json has CORES=x, use that - otherwise, use all cpus-1 (production) diff --git a/website/src/libs/api-v3/logger.js b/website/src/libs/api-v3/logger.js index 3ab20ad685..0d00ca7f4b 100644 --- a/website/src/libs/api-v3/logger.js +++ b/website/src/libs/api-v3/logger.js @@ -14,6 +14,7 @@ if (IS_PROD) { logger .add(winston.transports.Console, { colorize: true, + prettyPrint: true, }); } diff --git a/website/src/libs/api-v3/setupRoutes.js b/website/src/libs/api-v3/setupRoutes.js index 4bce7b1e26..8c346577bc 100644 --- a/website/src/libs/api-v3/setupRoutes.js +++ b/website/src/libs/api-v3/setupRoutes.js @@ -2,6 +2,7 @@ import fs from 'fs'; import path from 'path'; import express from 'express'; import _ from 'lodash'; + const CONTROLLERS_PATH = path.join(__dirname, '/../../controllers/api-v3/'); let router = express.Router(); // eslint-disable-line new-cap @@ -20,4 +21,4 @@ fs }); }); -export default router; \ No newline at end of file +export default router; diff --git a/website/src/middlewares/api-v3/index.js b/website/src/middlewares/api-v3/index.js index 0041e2dd54..dfd19e5493 100644 --- a/website/src/middlewares/api-v3/index.js +++ b/website/src/middlewares/api-v3/index.js @@ -5,9 +5,15 @@ import errorHandler from './errorHandler'; import bodyParser from 'body-parser'; import routes from '../../libs/api-v3/setupRoutes'; import notFoundHandler from './notFound'; +import nconf from 'nconf'; +import morgan from 'morgan'; + +const IS_PROD = nconf.get('IS_PROD'); +const DISABLE_LOGGING = nconf.get('DISABLE_REQUEST_LOGGING'); export default function attachMiddlewares (app) { - // Parse query parameters and json bodies + if (!IS_PROD && !DISABLE_LOGGING) app.use(morgan('dev')); + // TODO handle errors app.use(bodyParser.urlencoded({ extended: true, // Uses 'qs' library as old connect middleware @@ -15,7 +21,7 @@ export default function attachMiddlewares (app) { app.use(bodyParser.json()); app.use(analytics); - app.use(routes); + app.use('/api/v3', routes); app.use(notFoundHandler); // Error handler middleware, define as the last one diff --git a/website/src/server.js b/website/src/server.js index 21744c0813..aa0013d89e 100644 --- a/website/src/server.js +++ b/website/src/server.js @@ -89,7 +89,7 @@ app.use(domainMiddleware(server, mongoose)); // Matches all request except the ones going to /api/v3/** app.all(/^(?!\/api\/v3).+/i, oldApp); // Matches all requests going to /api/v3 -app.all('/api/v3', newApp); +app.all('/api/*', newApp); // Mount middlewares for the new app attachMiddlewares(newApp); From 3b633a87b94904cfbe196089fb2ced76f9a8a94f Mon Sep 17 00:00:00 2001 From: Matteo Pagliazzi Date: Wed, 18 Nov 2015 12:44:52 +0100 Subject: [PATCH 9/9] try fixing the notFound test --- test/api/v3/unit/middlewares/notFound.test.js | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/test/api/v3/unit/middlewares/notFound.test.js b/test/api/v3/unit/middlewares/notFound.test.js index f2f6082acf..44ca295e95 100644 --- a/test/api/v3/unit/middlewares/notFound.test.js +++ b/test/api/v3/unit/middlewares/notFound.test.js @@ -4,10 +4,11 @@ describe('notFound Middleware', () => { it('returns a 404 error when the resource is not found', () => { let request = requester().get('/api/v3/dummy-url'); - return expect(request) - .to.eventually.be.rejected.and.eql({ - error: "NotFound", - message: "Not found.", - }); - }); + return request.then((errBody) => { + expect(errBody.error).to.equal('NotFound'); + expect(errBody.message).to.equal('Not found.'); + }).to.eventually.be.rejected.and.eql({ + code: 404, + }); + }); });