Merge pull request #20 from Getable/error-on-invalid-where

Validate query and payload

README.md

@@ -11,6 +11,9 @@ npm install -S hapi-sequelize-crud
 
 ##Configure
 
+Please note that you should register `hapi-sequelize-crud` after defining your
+associations.
+
 ```javascript
 // First, register hapi-sequelize
 await register({
@@ -35,6 +38,7 @@ await register({
     // `models` property. If you omit this property, all models will have
     // models defined for them. e.g.
     models: ['cat', 'dog'] // only the cat and dog models will have routes created
+
     // or
     models: [
       // possible methods: list, get, scope, create, destroy, destroyAll, destroyScope, update
@@ -54,20 +58,59 @@ await register({
 ```
 
 ### Methods
-* list: get all rows in a table
+* **list**: get all rows in a table
-* get: get a single row
+* **get**: get a single row
-* scope: reference a [sequelize scope](http://docs.sequelizejs.com/en/latest/api/model/#scopeoptions-model)
+* **scope**: reference a [sequelize scope](http://docs.sequelizejs.com/en/latest/api/model/#scopeoptions-model)
-* create: create a new row
+* **create**: create a new row
-* destroy: delete a row
+* **destroy**: delete a row
-* destroyAll: delete all models in the table
+* **destroyAll**: delete all models in the table
-* destroyScope: use a [sequelize scope](http://docs.sequelizejs.com/en/latest/api/model/#scopeoptions-model) to find rows, then delete them
+* **destroyScope**: use a [sequelize scope](http://docs.sequelizejs.com/en/latest/api/model/#scopeoptions-model) to find rows, then delete them
-* update: update a row
+* **update**: update a row
 
+## `where` queries
+It's easy to restrict your requests using Sequelize's `where` query option. Just pass a query parameter.
 
-Please note that you should register `hapi-sequelize-crud` after defining your
+```js
-associations.
+// returns only teams that have a `city` property of "windsor"
+// GET /team?city=windsor
 
-##What do I get
+// results in the Sequelize query:
+Team.findOne({ where: { city: 'windsor' }})
+```
+
+You can also do more complex queries by setting the value of a key to JSON.
+
+```js
+// returns only teams that have a `address.city` property of "windsor"
+// GET /team?city={"address": "windsor"}
+// or
+// GET /team?city[address]=windsor
+
+// results in the Sequelize query:
+Team.findOne({ where: { address: { city: 'windsor' }}})
+```
+
+## `include` queries
+Getting related models is easy, just use a query parameter `include`.
+
+```js
+// returns all teams with their related City model
+// GET /teams?include=City
+
+// results in a Sequelize query:
+Team.findAll({include: City})
+```
+
+If you want to get multiple related models, just pass multiple `include` parameters.
+```js
+// returns all teams with their related City and Uniform models
+// GET /teams?include=City&include=Uniform
+
+// results in a Sequelize query:
+Team.findAll({include: [City, Uniform]})
+```
+
+## Full list of methods
 
 Let's say you have a `many-to-many` association like this:
 
@@ -82,8 +125,9 @@ You get these:
 # get an array of records
 GET /team/{id}/roles
 GET /role/{id}/teams
-# might also append query parameters to search for
+# might also append `where` query parameters to search for
 GET /role/{id}/teams?members=5
+GET /role/{id}/teams?city=healdsburg
 
 # you might also use scopes
 GET /teams/{scope}/roles/{scope}

package.json

@@ -25,25 +25,25 @@
   "license": "MIT",
   "devDependencies": {
     "ava": "^0.16.0",
-    "babel-cli": "^6.10.1",
+    "babel-cli": "^6.14.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.10.3",
+    "babel-plugin-transform-es2015-modules-commonjs": "^6.14.0",
-    "babel-preset-stage-1": "^6.5.0",
+    "babel-preset-stage-1": "^6.13.0",
     "eslint": "^3.4.0",
-    "eslint-config-pichak": "1.1.0",
+    "eslint-config-pichak": "^1.1.2",
     "eslint-plugin-ava": "^3.0.0",
-    "ghooks": "1.0.3",
+    "ghooks": "^1.3.2",
     "scripty": "^1.6.0",
     "sinon": "^1.17.5",
     "sinon-bluebird": "^3.0.2",
     "tap-xunit": "^1.4.0"
   },
   "dependencies": {
-    "boom": "^3.2.2",
+    "boom": "^4.0.0",
-    "joi": "7.2.1",
+    "joi": "^9.0.4",
-    "lodash": "4.0.0"
+    "lodash": "^4.15.0"
   },
   "optionalDependencies": {
     "babel-polyfill": "^6.13.0"

src/crud.js

@@ -5,10 +5,28 @@ import _ from 'lodash';
 import { parseInclude, parseWhere } from './utils';
 import { notFound } from 'boom';
 import * as associations from './associations/index';
+import getConfigForMethod from './get-config-for-method.js';
 
-const createAll = ({ server, model, prefix, config }) => {
+const createAll = ({
+  server,
+  model,
+  prefix,
+  config,
+  attributeValidation,
+  associationValidation,
+}) => {
   Object.keys(methods).forEach((method) => {
-    methods[method]({ server, model, prefix, config });
+    methods[method]({
+      server,
+      model,
+      prefix,
+      config: getConfigForMethod({
+        method,
+        attributeValidation,
+        associationValidation,
+        config,
+      }),
+    });
   });
 };
 
@@ -34,13 +52,43 @@ models: {
 
 export default (server, model, { prefix, defaultConfig: config, models: permissions }) => {
   const modelName = model._singular;
+  const modelAttributes = Object.keys(model.attributes);
+  const modelAssociations = Object.keys(model.associations);
 
+  const attributeValidation = modelAttributes.reduce((params, attribute) => {
+    params[attribute] = joi.any();
+    return params;
+  }, {});
+
+  const associationValidation = {
+    include: joi.array().items(joi.string().valid(...modelAssociations)),
+  };
+
+  // if we don't have any permissions set, just create all the methods
   if (!permissions) {
-    createAll({ server, model, prefix, config });
+    createAll({
+      server,
+      model,
+      prefix,
+      config,
+      attributeValidation,
+      associationValidation,
+    });
+  // if permissions are set, but we can't parse them, throw an error
   } else if (!Array.isArray(permissions)) {
     throw new Error('hapi-sequelize-crud: `models` property must be an array');
+  // if permissions are set, but the only thing we've got is a model name, there
+  // are no permissions to be set, so just create all methods and move on
   } else if (permissions.includes(modelName)) {
-    createAll({ server, model, prefix, config });
+    createAll({
+      server,
+      model,
+      prefix,
+      config,
+      attributeValidation,
+      associationValidation,
+    });
+  // if we've gotten here, we have complex permissions and need to set them
   } else {
     const permissionOptions = permissions.filter((permission) => {
       return permission.model === modelName;
@@ -56,11 +104,23 @@ export default (server, model, { prefix, defaultConfig: config, models: permissi
               server,
               model,
               prefix,
-              config: permissionConfig,
+              config: getConfigForMethod({
+                method,
+                attributeValidation,
+                associationValidation,
+                config: permissionConfig,
+              }),
             });
           });
         } else {
-          createAll({ server, model, prefix, config: permissionConfig });
+          createAll({
+            server,
+            model,
+            prefix,
+            attributeValidation,
+            associationValidation,
+            config: permissionConfig,
+          });
         }
       }
     });
@@ -102,19 +162,21 @@ export const get = ({ server, model, prefix = '/', config }) => {
       const { id } = request.params;
       if (id) where[model.primaryKeyField] = id;
 
+      if (include instanceof Error) return void reply(include);
+
       const instance = await model.findOne({ where, include });
 
       if (!instance) return void reply(notFound(`${id} not found.`));
 
       reply(instance);
     },
-    config: _.defaultsDeep({
+    config: _.defaultsDeep(config, {
       validate: {
-        params: joi.object().keys({
+        params: {
           id: joi.any(),
-        }),
+        },
       },
-    }, config),
+    }),
   });
 };
 
@@ -130,17 +192,19 @@ export const scope = ({ server, model, prefix = '/', config }) => {
       const include = parseInclude(request);
       const where = parseWhere(request);
 
+      if (include instanceof Error) return void reply(include);
+
       const list = await model.scope(request.params.scope).findAll({ include, where });
 
       reply(list);
     },
-    config: _.defaultsDeep({
+    config: _.defaultsDeep(config, {
       validate: {
-        params: joi.object().keys({
+        params: {
           scope: joi.string().valid(...scopes),
-        }),
+        },
       },
-    }, config),
+    }),
   });
 };
 
@@ -213,19 +277,21 @@ export const destroyScope = ({ server, model, prefix = '/', config }) => {
       const include = parseInclude(request);
       const where = parseWhere(request);
 
+      if (include instanceof Error) return void reply(include);
+
       const list = await model.scope(request.params.scope).findAll({ include, where });
 
       await Promise.all(list.map(instance => instance.destroy()));
 
       reply(list);
     },
-    config: _.defaultsDeep({
+    config: _.defaultsDeep(config, {
       validate: {
-        params: joi.object().keys({
+        params: {
           scope: joi.string().valid(...scopes),
-        }),
+        },
       },
-    }, config),
+    }),
   });
 };
 
@@ -237,11 +303,7 @@ export const update = ({ server, model, prefix = '/', config }) => {
     @error
     async handler(request, reply) {
       const { id } = request.params;
-      const instance = await model.findOne({
+      const instance = await model.findById(id);
-        where: {
-          id,
-        },
-      });
 
       if (!instance) return void reply(notFound(`${id} not found.`));
 
@@ -250,11 +312,14 @@ export const update = ({ server, model, prefix = '/', config }) => {
       reply(instance);
     },
 
-    config: _.defaultsDeep({
+    config: _.defaultsDeep(config, {
       validate: {
         payload: joi.object().required(),
+        params: {
+          id: joi.any(),
+        },
       },
-    }, config),
+    }),
   });
 };
 

src/get-config-for-method.js

@@ -0,0 +1,88 @@
+import { defaultsDeep } from 'lodash';
+import joi from 'joi';
+
+export const sequelizeOperators = {
+  $and: joi.any(),
+  $or: joi.any(),
+  $gt: joi.any(),
+  $gte: joi.any(),
+  $lt: joi.any(),
+  $lte: joi.any(),
+  $ne: joi.any(),
+  $eq: joi.any(),
+  $not: joi.any(),
+  $between: joi.any(),
+  $notBetween: joi.any(),
+  $in: joi.any(),
+  $notIn: joi.any(),
+  $like: joi.any(),
+  $notLike: joi.any(),
+  $iLike: joi.any(),
+  $notILike: joi.any(),
+  $overlap: joi.any(),
+  $contains: joi.any(),
+  $contained: joi.any(),
+  $any: joi.any(),
+  $col: joi.any(),
+};
+
+export const whereMethods = [
+  'list',
+  'get',
+  'scope',
+  'destroy',
+  'destoryScope',
+  'destroyAll',
+];
+
+export const includeMethods = [
+  'list',
+  'get',
+  'scope',
+  'destoryScope',
+];
+
+export const payloadMethods = [
+  'create',
+  'update',
+];
+
+export default ({ method, attributeValidation, associationValidation, config = {} }) => {
+  const hasWhere = whereMethods.includes(method);
+  const hasInclude = includeMethods.includes(method);
+  const hasPayload = payloadMethods.includes(method);
+  const methodConfig = { ...config };
+
+  if (hasWhere) {
+    defaultsDeep(methodConfig, {
+      validate: {
+        query: {
+          ...attributeValidation,
+          ...sequelizeOperators,
+        },
+      },
+    });
+  }
+
+  if (hasInclude) {
+    defaultsDeep(methodConfig, {
+      validate: {
+        query: {
+          ...associationValidation,
+        },
+      },
+    });
+  }
+
+  if (hasPayload) {
+    defaultsDeep(methodConfig, {
+      validate: {
+        payload: {
+          ...attributeValidation,
+        },
+      },
+    });
+  }
+
+  return methodConfig;
+};

src/get-config-for-method.test.js

@@ -0,0 +1,117 @@
+import test from 'ava';
+import joi from 'joi';
+import
+  getConfigForMethod, {
+  whereMethods,
+  includeMethods,
+  payloadMethods,
+  sequelizeOperators,
+} from './get-config-for-method.js';
+
+test.beforeEach((t) => {
+  t.context.attributeValidation = {
+    myKey: joi.any(),
+  };
+
+  t.context.associationValidation = {
+    include: ['MyModel'],
+  };
+
+  t.context.config = {
+    cors: {},
+  };
+});
+
+test('get-config-for-method validate.query seqeulizeOperators', (t) => {
+  whereMethods.forEach((method) => {
+    const configForMethod = getConfigForMethod({ method });
+    const { query } = configForMethod.validate;
+    const configForMethodValidateQueryKeys = Object.keys(query);
+
+    t.truthy(
+      query,
+      `applies query validation for ${method}`
+    );
+
+    Object.keys(sequelizeOperators).forEach((operator) => {
+      t.truthy(
+        configForMethodValidateQueryKeys.includes(operator),
+        `applies sequelize operator "${operator}" in validate.where for ${method}`
+      );
+    });
+  });
+});
+
+test('get-config-for-method validate.query attributeValidation', (t) => {
+  const { attributeValidation } = t.context;
+
+  whereMethods.forEach((method) => {
+    const configForMethod = getConfigForMethod({ method, attributeValidation });
+    const { query } = configForMethod.validate;
+
+    Object.keys(attributeValidation).forEach((key) => {
+      t.truthy(
+        query[key]
+        , `applies attributeValidation (${key}) to validate.query`
+      );
+    });
+  });
+});
+
+test('get-config-for-method validate.query associationValidation', (t) => {
+  const { attributeValidation, associationValidation } = t.context;
+
+  includeMethods.forEach((method) => {
+    const configForMethod = getConfigForMethod({
+      method,
+      attributeValidation,
+      associationValidation,
+    });
+    const { query } = configForMethod.validate;
+
+    Object.keys(attributeValidation).forEach((key) => {
+      t.truthy(
+        query[key]
+        , `applies attributeValidation (${key}) to validate.query when include should be applied`
+      );
+    });
+
+    Object.keys(associationValidation).forEach((key) => {
+      t.truthy(
+        query[key]
+        , `applies associationValidation (${key}) to validate.query when include should be applied`
+      );
+    });
+  });
+});
+
+test('get-config-for-method validate.payload associationValidation', (t) => {
+  const { attributeValidation } = t.context;
+
+  payloadMethods.forEach((method) => {
+    const configForMethod = getConfigForMethod({ method, attributeValidation });
+    const { payload } = configForMethod.validate;
+
+    Object.keys(attributeValidation).forEach((key) => {
+      t.truthy(
+        payload[key]
+        , `applies attributeValidation (${key}) to validate.payload`
+      );
+    });
+  });
+});
+
+test('get-config-for-method does not modify initial config on multiple passes', (t) => {
+  const { config } = t.context;
+  const originalConfig = { ...config };
+
+  whereMethods.forEach((method) => {
+    getConfigForMethod({ method, config });
+  });
+
+  t.deepEqual(
+    config
+    , originalConfig
+    , 'does not modify the original config object'
+  );
+});