From ddc6fcceb8004b27524e0594b50ee4afce1aeda8 Mon Sep 17 00:00:00 2001 From: Joey Baker Date: Wed, 28 Sep 2016 21:16:17 -0700 Subject: [PATCH 1/5] Chore (build) set sourcemaps to inline This ensures that node can read the sourcemaps and provide useful stacktraces --- .babelrc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.babelrc b/.babelrc index 455797a..dbcf8f2 100644 --- a/.babelrc +++ b/.babelrc @@ -10,5 +10,5 @@ "transform-decorators-legacy", "transform-es2015-modules-commonjs" ], - "sourceMaps": true + "sourceMaps": "inline" } From 3c516aa604cf9c3d752486bacd44958d21483ab7 Mon Sep 17 00:00:00 2001 From: Joey Baker Date: Wed, 28 Sep 2016 21:16:38 -0700 Subject: [PATCH 2/5] Chore gitignore mac junk files --- .gitignore | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.gitignore b/.gitignore index 2d7cacb..e1c0e0e 100644 --- a/.gitignore +++ b/.gitignore @@ -33,3 +33,6 @@ node_modules # Debug log from npm npm-debug.log + +# System +.DS_Store From adb1d7198459d78b9ac2a5c12321ed1551791fb2 Mon Sep 17 00:00:00 2001 From: Joey Baker Date: Thu, 20 Oct 2016 16:47:04 -0700 Subject: [PATCH 3/5] Chore(deps) update patches and minors --- package.json | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/package.json b/package.json index 7b4d1ee..eb4aa84 100644 --- a/package.json +++ b/package.json @@ -25,26 +25,26 @@ "license": "MIT", "devDependencies": { "ava": "^0.16.0", - "babel-cli": "^6.14.0", + "babel-cli": "^6.16.0", "babel-plugin-add-module-exports": "^0.2.1", "babel-plugin-closure-elimination": "^1.0.6", "babel-plugin-transform-decorators-legacy": "^1.3.4", - "babel-plugin-transform-es2015-modules-commonjs": "^6.14.0", - "babel-preset-stage-1": "^6.13.0", - "babel-register": "^6.14.0", - "eslint": "^3.4.0", + "babel-plugin-transform-es2015-modules-commonjs": "^6.16.0", + "babel-preset-stage-1": "^6.16.0", + "babel-register": "^6.16.3", + "eslint": "^3.8.1", "eslint-config-pichak": "^1.1.2", - "eslint-plugin-ava": "^3.0.0", + "eslint-plugin-ava": "^3.1.1", "ghooks": "^1.3.2", "scripty": "^1.6.0", - "sinon": "^1.17.5", - "sinon-bluebird": "^3.0.2", + "sinon": "^1.17.6", + "sinon-bluebird": "^3.1.0", "tap-xunit": "^1.4.0" }, "dependencies": { - "boom": "^4.0.0", - "joi": "^9.0.4", - "lodash": "^4.15.0" + "boom": "^4.2.0", + "joi": "^9.2.0", + "lodash": "^4.16.4" }, "optionalDependencies": { "babel-polyfill": "^6.13.0" From 5923f0dbcb49604abe19ae36a1b469ee78685467 Mon Sep 17 00:00:00 2001 From: Joey Baker Date: Thu, 20 Oct 2016 16:50:26 -0700 Subject: [PATCH 4/5] Test(crud) ensure list doesn't error --- src/crud.test.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/crud.test.js b/src/crud.test.js index 53c16c5..317a9f8 100644 --- a/src/crud.test.js +++ b/src/crud.test.js @@ -126,6 +126,8 @@ test('crud#list handler', async (t) => { const response = reply.args[0][0]; + t.falsy(response instanceof Error, response); + t.deepEqual( response, models.map(({ id }) => ({ id })), From 8966d7b287422734b76e87fe5cc57eb29d4416bc Mon Sep 17 00:00:00 2001 From: Joey Baker Date: Wed, 28 Sep 2016 21:17:16 -0700 Subject: [PATCH 5/5] Feat add support of limit, offset, order Allows passing these as query params to list and scope methods. --- README.md | 39 +++++++++++ src/crud.js | 12 +++- src/crud.test.js | 74 ++++++++++++++++++- src/get-config-for-method.js | 19 +++++ src/get-config-for-method.test.js | 96 +++++++++++++++++++++++++ src/utils.js | 44 ++++++++++-- src/utils.test.js | 113 ++++++++++++++++++++++++++++++ 7 files changed, 389 insertions(+), 8 deletions(-) create mode 100644 src/utils.test.js diff --git a/README.md b/README.md index bff8327..53746bc 100644 --- a/README.md +++ b/README.md @@ -124,6 +124,45 @@ If you want to get multiple related models, just pass multiple `include` paramet Team.findAll({include: [City, Uniform]}) ``` +## `limit` and `offset` queries +Restricting list (`GET`) and scope queries to a restricted count can be done by passing `limit=` and/or `offset=`. + +```js +// returns 10 teams starting from the 10th +// GET /teams?limit=10&offset=10 + +// results in a Sequelize query: +Team.findAll({limit: 10, offset: 10}) +``` + +## `order` queries +You can change the order of the resulting query by passing `order` to the query. + +```js +// returns the teams ordered by the name column +// GET /teams?order[]=name + +// results in a Sequelize query: +Team.findAll({order: ['name']}) +``` + +```js +// returns the teams ordered by the name column, descending +// GET /teams?order[0]=name&order[0]=DESC + +// results in a Sequelize query: +Team.findAll({order: [['name', 'DESC']]}) +``` + +```js +// returns the teams ordered by the name, then the city columns, descending +// GET /teams?order[0]=name&order[1]=city + +// results in a Sequelize query: +Team.findAll({order: [['name'], ['city']]}) +``` + + ## Authorization and other hooks You can use Hapi's [`ext` option](http://hapijs.com/api#route-options) to interact with the request both before and after this module does. This is useful if you want to enforce authorization, or modify the request before or after this module does. Hapi [has a full list of hooks](http://hapijs.com/api#request-lifecycle) you can use. diff --git a/src/crud.js b/src/crud.js index 5d4777b..7ac31fc 100644 --- a/src/crud.js +++ b/src/crud.js @@ -2,7 +2,7 @@ import joi from 'joi'; import path from 'path'; import error from './error'; import _ from 'lodash'; -import { parseInclude, parseWhere } from './utils'; +import { parseInclude, parseWhere, parseLimitAndOffset, parseOrder } from './utils'; import { notFound } from 'boom'; import * as associations from './associations/index'; import getConfigForMethod from './get-config-for-method.js'; @@ -144,11 +144,13 @@ export const list = ({ server, model, prefix = '/', config }) => { async handler(request, reply) { const include = parseInclude(request); const where = parseWhere(request); + const { limit, offset } = parseLimitAndOffset(request); + const order = parseOrder(request); if (include instanceof Error) return void reply(include); const list = await model.findAll({ - where, include, + where, include, limit, offset, order, }); reply(list.map((item) => item.toJSON())); @@ -191,10 +193,14 @@ export const scope = ({ server, model, prefix = '/', config }) => { async handler(request, reply) { const include = parseInclude(request); const where = parseWhere(request); + const { limit, offset } = parseLimitAndOffset(request); + const order = parseOrder(request); if (include instanceof Error) return void reply(include); - const list = await model.scope(request.params.scope).findAll({ include, where }); + const list = await model.scope(request.params.scope).findAll({ + include, where, limit, offset, order, + }); reply(list.map((item) => item.toJSON())); }, diff --git a/src/crud.test.js b/src/crud.test.js index 317a9f8..0856451 100644 --- a/src/crud.test.js +++ b/src/crud.test.js @@ -37,7 +37,7 @@ test.beforeEach('setup request stub', (t) => { t.context.request = { query: {}, payload: {}, - models: [t.context.model], + models: t.context.models, }; }); @@ -157,3 +157,75 @@ test('crud#list handler if parseInclude errors', async (t) => { 'responds with a Boom error' ); }); + +test('crud#list handler with limit', async (t) => { + const { server, model, request, reply, models } = t.context; + const { findAll } = model; + + // set the limit + request.query.limit = 1; + + list({ server, model }); + const { handler } = server.route.args[0][0]; + model.findAll.resolves(models); + + try { + await handler(request, reply); + } catch (e) { + t.ifError(e, 'does not error while handling'); + } finally { + t.pass('does not error while handling'); + } + + t.truthy( + reply.calledOnce + , 'calls reply only once' + ); + + const response = reply.args[0][0]; + const findAllArgs = findAll.args[0][0]; + + t.falsy(response instanceof Error, response); + + t.is( + findAllArgs.limit, + request.query.limit, + 'queries with the limit' + ); +}); + +test('crud#list handler with order', async (t) => { + const { server, model, request, reply, models } = t.context; + const { findAll } = model; + + // set the limit + request.query.order = 'key'; + + list({ server, model }); + const { handler } = server.route.args[0][0]; + model.findAll.resolves(models); + + try { + await handler(request, reply); + } catch (e) { + t.ifError(e, 'does not error while handling'); + } finally { + t.pass('does not error while handling'); + } + + t.truthy( + reply.calledOnce + , 'calls reply only once' + ); + + const response = reply.args[0][0]; + const findAllArgs = findAll.args[0][0]; + + t.falsy(response instanceof Error, response); + + t.deepEqual( + findAllArgs.order, + [request.query.order], + 'queries with the order as an array b/c that\'s what sequelize wants' + ); +}); diff --git a/src/get-config-for-method.js b/src/get-config-for-method.js index c9a318a..a87a43e 100644 --- a/src/get-config-for-method.js +++ b/src/get-config-for-method.js @@ -66,6 +66,11 @@ export const idParamsMethods = [ 'update', ]; +export const restrictMethods = [ + 'list', + 'scope', +]; + export default ({ method, attributeValidation, associationValidation, scopes = [], config = {}, }) => { @@ -74,6 +79,7 @@ export default ({ const hasPayload = payloadMethods.includes(method); const hasScopeParams = scopeParamsMethods.includes(method); const hasIdParams = idParamsMethods.includes(method); + const hasRestrictMethods = restrictMethods.includes(method); // clone the config so we don't modify it on multiple passes. let methodConfig = { ...config, validate: { ...config.validate } }; @@ -133,5 +139,18 @@ export default ({ methodConfig = set(methodConfig, 'validate.params', params); } + if (hasRestrictMethods) { + const query = concatToJoiObject(joi.object() + .keys({ + limit: joi.number().min(0).integer(), + offset: joi.number().min(0).integer(), + order: joi.array(), + }), + get(methodConfig, 'validate.query') + ); + + methodConfig = set(methodConfig, 'validate.query', query); + } + return methodConfig; }; diff --git a/src/get-config-for-method.test.js b/src/get-config-for-method.test.js index 284ddeb..ea8edad 100644 --- a/src/get-config-for-method.test.js +++ b/src/get-config-for-method.test.js @@ -7,6 +7,7 @@ import payloadMethods, scopeParamsMethods, idParamsMethods, + restrictMethods, sequelizeOperators, } from './get-config-for-method.js'; @@ -468,6 +469,101 @@ test('validate.payload idParamsMethods', (t) => { }); }); +test('validate.query restrictMethods', (t) => { + restrictMethods.forEach((method) => { + const configForMethod = getConfigForMethod({ method }); + const { query } = configForMethod.validate; + const restrictKeys = ['limit', 'offset']; + + restrictKeys.forEach((key) => { + t.ifError( + query.validate({ [key]: 0 }).error + , `applies restriction (${key}) to validate.query` + ); + }); + + t.ifError( + query.validate({ order: ['thing', 'DESC'] }).error + , 'applies `order` to validate.query' + ); + + t.truthy( + query.validate({ notAThing: true }).error + , 'errors on a non-valid key' + ); + }); +}); + +test('validate.query restrictMethods w/ config as plain object', (t) => { + const config = { + validate: { + query: { + aKey: joi.boolean(), + }, + }, + }; + + restrictMethods.forEach((method) => { + const configForMethod = getConfigForMethod({ + method, + config, + }); + const { query } = configForMethod.validate; + + const keys = [ + ...Object.keys(config.validate.query), + ]; + + keys.forEach((key) => { + t.ifError( + query.validate({ [key]: true }).error + , `applies ${key} to validate.query` + ); + }); + + t.truthy( + query.validate({ notAThing: true }).error + , 'errors on a non-valid key' + ); + }); +}); + +test('validate.query restrictMethods w/ config as joi object', (t) => { + const queryKeys = { + aKey: joi.boolean(), + }; + const config = { + validate: { + query: joi.object().keys(queryKeys), + }, + }; + + whereMethods.forEach((method) => { + const configForMethod = getConfigForMethod({ + method, + config, + }); + const { query } = configForMethod.validate; + + const keys = [ + ...Object.keys(queryKeys), + ]; + + keys.forEach((key) => { + t.ifError( + query.validate({ [key]: true }).error + , `applies ${key} to validate.query` + ); + }); + + t.truthy( + query.validate({ notAThing: true }).error + , 'errors on a non-valid key' + ); + }); +}); + + test('does not modify initial config on multiple passes', (t) => { const { config } = t.context; const originalConfig = { ...config }; diff --git a/src/utils.js b/src/utils.js index 880cf8b..7ca006b 100644 --- a/src/utils.js +++ b/src/utils.js @@ -1,9 +1,13 @@ -import { omit, identity } from 'lodash'; +import { omit, identity, toNumber, isString, isUndefined } from 'lodash'; import { notImplemented } from 'boom'; +const sequelizeKeys = ['include', 'order', 'limit', 'offset']; + export const parseInclude = request => { - const include = Array.isArray(request.query.include) ? request.query.include - : [request.query.include]; + const include = Array.isArray(request.query.include) + ? request.query.include + : [request.query.include] + ; const noGetDb = typeof request.getDb !== 'function'; const noRequestModels = !request.models; @@ -27,7 +31,7 @@ export const parseInclude = request => { }; export const parseWhere = request => { - const where = omit(request.query, 'include'); + const where = omit(request.query, sequelizeKeys); for (const key of Object.keys(where)) { try { @@ -40,6 +44,38 @@ export const parseWhere = request => { return where; }; +export const parseLimitAndOffset = (request) => { + const { limit, offset } = request.query; + const out = {}; + if (!isUndefined(limit)) { + out.limit = toNumber(limit); + } + if (!isUndefined(offset)) { + out.offset = toNumber(offset); + } + return out; +}; + +export const parseOrder = (request) => { + const { order } = request.query; + + if (!order) return null; + + // transform to an array so sequelize will escape the input for us and + // maintain security. See http://docs.sequelizejs.com/en/latest/docs/querying/#ordering + if (isString(order)) return order.split(' '); + + for (const key of Object.keys(order)) { + try { + order[key] = JSON.parse(order[key]); + } catch (e) { + // + } + } + + return order; +}; + export const getMethod = (model, association, plural = true, method = 'get') => { const a = plural ? association.original.plural : association.original.singular; const b = plural ? association.original.singular : association.original.plural; // alternative diff --git a/src/utils.test.js b/src/utils.test.js new file mode 100644 index 0000000..43eb29e --- /dev/null +++ b/src/utils.test.js @@ -0,0 +1,113 @@ +import test from 'ava'; +import { parseLimitAndOffset, parseOrder, parseWhere } from './utils.js'; + +test.beforeEach((t) => { + t.context.request = { query: {} }; +}); + +test('parseLimitAndOffset is a function', (t) => { + t.is(typeof parseLimitAndOffset, 'function'); +}); + +test('parseLimitAndOffset returns limit and offset', (t) => { + const { request } = t.context; + request.query.limit = 1; + request.query.offset = 2; + request.query.thing = 'hi'; + + t.is( + parseLimitAndOffset(request).limit + , request.query.limit + ); + + t.is( + parseLimitAndOffset(request).offset + , request.query.offset + ); +}); + +test('parseLimitAndOffset returns limit and offset as numbers', (t) => { + const { request } = t.context; + const limit = 1; + const offset = 2; + request.query.limit = `${limit}`; + request.query.offset = `${offset}`; + request.query.thing = 'hi'; + + t.is( + parseLimitAndOffset(request).limit + , limit + ); + + t.is( + parseLimitAndOffset(request).offset + , offset + ); +}); + +test('parseOrder is a function', (t) => { + t.is(typeof parseOrder, 'function'); +}); + +test('parseOrder returns order when a string', (t) => { + const { request } = t.context; + const order = 'thing'; + request.query.order = order; + request.query.thing = 'hi'; + + t.deepEqual( + parseOrder(request) + , [order] + ); +}); + +test('parseOrder returns order when json', (t) => { + const { request } = t.context; + const order = [{ model: 'User' }, 'DESC']; + request.query.order = [JSON.stringify({ model: 'User' }), 'DESC']; + request.query.thing = 'hi'; + + t.deepEqual( + parseOrder(request) + , order + ); +}); + +test('parseOrder returns null when not defined', (t) => { + const { request } = t.context; + request.query.thing = 'hi'; + + t.is( + parseOrder(request) + , null + ); +}); + + +test('parseWhere is a function', (t) => { + t.is(typeof parseWhere, 'function'); +}); + +test('parseWhere returns the non-sequelize keys', (t) => { + const { request } = t.context; + request.query.order = 'thing'; + request.query.include = 'User'; + request.query.limit = 2; + request.query.thing = 'hi'; + + t.deepEqual( + parseWhere(request) + , { thing: 'hi' } + ); +}); + +test('parseWhere returns json converted keys', (t) => { + const { request } = t.context; + request.query.order = 'hi'; + request.query.thing = '{"id": {"$in": [2, 3]}}'; + + t.deepEqual( + parseWhere(request) + , { thing: { id: { $in: [2, 3] } } } + ); +});