Merge branch 'master' of github.com:mdibaiee/hapi-sequelize-crud

.eslintrc

@@ -1,3 +1,9 @@
 {
-  "extends": "pichak"
+  "plugins": [
+    "ava"
+  ],
+  "extends": [
+    "pichak",
+    "plugin:ava/recommended"
+  ]
 }

circle.yml

@@ -0,0 +1,9 @@
+machine:
+  node:
+    version: 6.5.0
+
+dependencies:
+  pre:
+    - npm prune
+  post:
+    - mkdir -p $CIRCLE_TEST_REPORTS/ava

package.json

@@ -1,6 +1,6 @@
 {
   "name": "hapi-sequelize-crud",
-  "version": "2.5.3",
+  "version": "2.5.4",
   "description": "Hapi plugin that automatically generates RESTful API for CRUD",
   "main": "build/index.js",
   "config": {
@@ -9,8 +9,9 @@
     }
   },
   "scripts": {
-    "lint": "eslint src test",
+    "lint": "eslint src",
-    "test": "echo \"Error: no test specified\" && exit 1",
+    "test": "ava --require babel-register --source='*.test.js' --tap=${CI-false} | $(if [ -z ${CI:-} ]; then echo 'tail'; else tap-xunit > $CIRCLE_TEST_REPORTS/ava/ava.xml; fi;)",
+    "tdd": "ava --require babel-register --source='*.test.js' --watch",
     "build": "scripty",
     "watch": "scripty"
   },
@@ -23,16 +24,21 @@
   "author": "Mahdi Dibaiee <mdibaiee@aol.com> (http://dibaiee.ir/)",
   "license": "MIT",
   "devDependencies": {
+    "ava": "^0.16.0",
     "babel-cli": "^6.10.1",
     "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-preset-stage-1": "^6.5.0",
-    "eslint": "2.10.2",
+    "eslint": "^3.4.0",
     "eslint-config-pichak": "1.1.0",
+    "eslint-plugin-ava": "^3.0.0",
     "ghooks": "1.0.3",
-    "scripty": "^1.6.0"
+    "scripty": "^1.6.0",
+    "sinon": "^1.17.5",
+    "sinon-bluebird": "^3.0.2",
+    "tap-xunit": "^1.4.0"
   },
   "dependencies": {
     "boom": "^3.2.2",

src/crud.js

@@ -1,4 +1,5 @@
 import joi from 'joi';
+import path from 'path';
 import error from './error';
 import _ from 'lodash';
 import { parseInclude, parseWhere } from './utils';
@@ -66,10 +67,10 @@ export default (server, model, { prefix, defaultConfig: config, models: permissi
   }
 };
 
-export const list = ({ server, model, prefix, config }) => {
+export const list = ({ server, model, prefix = '/', config }) => {
   server.route({
     method: 'GET',
-    path: `${prefix}/${model._plural}`,
+    path: path.join(prefix, model._plural),
 
     @error
     async handler(request, reply) {
@@ -89,10 +90,10 @@ export const list = ({ server, model, prefix, config }) => {
   });
 };
 
-export const get = ({ server, model, prefix, config }) => {
+export const get = ({ server, model, prefix = '/', config }) => {
   server.route({
     method: 'GET',
-    path: `${prefix}/${model._singular}/{id?}`,
+    path: path.join(prefix, model._singular, '{id?}'),
 
     @error
     async handler(request, reply) {
@@ -117,12 +118,12 @@ export const get = ({ server, model, prefix, config }) => {
   });
 };
 
-export const scope = ({ server, model, prefix, config }) => {
+export const scope = ({ server, model, prefix = '/', config }) => {
   const scopes = Object.keys(model.options.scopes);
 
   server.route({
     method: 'GET',
-    path: `${prefix}/${model._plural}/{scope}`,
+    path: path.join(prefix, model._plural, '{scope}'),
 
     @error
     async handler(request, reply) {
@@ -143,10 +144,10 @@ export const scope = ({ server, model, prefix, config }) => {
   });
 };
 
-export const create = ({ server, model, prefix, config }) => {
+export const create = ({ server, model, prefix = '/', config }) => {
   server.route({
     method: 'POST',
-    path: `${prefix}/${model._singular}`,
+    path: path.join(prefix, model._singular),
 
     @error
     async handler(request, reply) {
@@ -159,10 +160,10 @@ export const create = ({ server, model, prefix, config }) => {
   });
 };
 
-export const destroy = ({ server, model, prefix, config }) => {
+export const destroy = ({ server, model, prefix = '/', config }) => {
   server.route({
     method: 'DELETE',
-    path: `${prefix}/${model._singular}/{id?}`,
+    path: path.join(prefix, model._singular, '{id?}'),
 
     @error
     async handler(request, reply) {
@@ -180,10 +181,10 @@ export const destroy = ({ server, model, prefix, config }) => {
   });
 };
 
-export const destroyAll = ({ server, model, prefix, config }) => {
+export const destroyAll = ({ server, model, prefix = '/', config }) => {
   server.route({
     method: 'DELETE',
-    path: `${prefix}/${model._plural}`,
+    path: path.join(prefix, model._plural),
 
     @error
     async handler(request, reply) {
@@ -200,12 +201,12 @@ export const destroyAll = ({ server, model, prefix, config }) => {
   });
 };
 
-export const destroyScope = ({ server, model, prefix, config }) => {
+export const destroyScope = ({ server, model, prefix = '/', config }) => {
   const scopes = Object.keys(model.options.scopes);
 
   server.route({
     method: 'DELETE',
-    path: `${prefix}/${model._plural}/{scope}`,
+    path: path.join(prefix, model._plural, '{scope}'),
 
     @error
     async handler(request, reply) {
@@ -228,10 +229,10 @@ export const destroyScope = ({ server, model, prefix, config }) => {
   });
 };
 
-export const update = ({ server, model, prefix, config }) => {
+export const update = ({ server, model, prefix = '/', config }) => {
   server.route({
     method: 'PUT',
-    path: `${prefix}/${model._singular}/{id}`,
+    path: path.join(prefix, model._singular, '{id}'),
 
     @error
     async handler(request, reply) {

src/crud.test.js

@@ -0,0 +1,146 @@
+import test from 'ava';
+import { list } from './crud.js';
+import { stub } from 'sinon';
+import 'sinon-bluebird';
+
+const METHODS = {
+  GET: 'GET',
+};
+
+test.beforeEach('setup server', (t) => {
+  t.context.server = {
+    route: stub(),
+  };
+});
+
+test.beforeEach('setup model', (t) => {
+  t.context.model = {
+    findAll: stub(),
+    _plural: 'models',
+    _singular: 'model',
+  };
+});
+
+test.beforeEach('setup request stub', (t) => {
+  t.context.request = {
+    query: {},
+    payload: {},
+    models: [t.context.model],
+  };
+});
+
+test.beforeEach('setup reply stub', (t) => {
+  t.context.reply = stub();
+});
+
+test('crud#list without prefix', (t) => {
+  const { server, model } = t.context;
+
+  list({ server, model });
+  const { path } = server.route.args[0][0];
+
+  t.falsy(
+    path.includes('undefined'),
+    'correctly sets the path without a prefix defined',
+  );
+
+  t.is(
+    path,
+    `/${model._plural}`,
+    'the path sets to the plural model'
+  );
+});
+
+test('crud#list with prefix', (t) => {
+  const { server, model } = t.context;
+  const prefix = '/v1';
+
+  list({ server, model, prefix });
+  const { path } = server.route.args[0][0];
+
+  t.is(
+    path,
+    `${prefix}/${model._plural}`,
+    'the path sets to the plural model with the prefix'
+  );
+});
+
+test('crud#list method', (t) => {
+  const { server, model } = t.context;
+
+  list({ server, model });
+  const { method } = server.route.args[0][0];
+
+  t.is(
+    method,
+    METHODS.GET,
+    `sets the method to ${METHODS.GET}`
+  );
+});
+
+test('crud#list config', (t) => {
+  const { server, model } = t.context;
+  const userConfig = {};
+
+  list({ server, model, config: userConfig });
+  const { config } = server.route.args[0][0];
+
+  t.is(
+    config,
+    userConfig,
+    'sets the user config'
+  );
+});
+
+test('crud#list handler', async (t) => {
+  const { server, model, request, reply } = t.context;
+  const allModels = [{ id: 1 }, { id: 2 }];
+
+  list({ server, model });
+  const { handler } = server.route.args[0][0];
+  model.findAll.resolves(allModels);
+
+  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];
+
+  t.is(
+    response,
+    allModels,
+    'responds with the list of models'
+  );
+});
+
+test('crud#list handler if parseInclude errors', async (t) => {
+  const { server, model, request, reply } = t.context;
+  // we _want_ the error
+  delete request.models;
+
+  list({ server, model });
+  const { handler } = server.route.args[0][0];
+
+  await handler(request, reply);
+
+  t.truthy(
+    reply.calledOnce
+    , 'calls reply only once'
+  );
+
+  const response = reply.args[0][0];
+
+  t.truthy(
+    response.isBoom,
+    'responds with a Boom error'
+  );
+});

src/error.js

@@ -8,11 +8,20 @@ export default (target, key, descriptor) => {
       await fn(request, reply);
     } catch (e) {
       if (e.original) {
-        const { code, detail } = e.original;
+        const { code, detail, hint } = e.original;
+        let error;
 
         // pg error codes https://www.postgresql.org/docs/9.5/static/errcodes-appendix.html
         if (code && (code.startsWith('22') || code.startsWith('23'))) {
-          const error = Boom.wrap(e, 406);
+          error = Boom.wrap(e, 406);
+        } else if (code && (code.startsWith('42'))) {
+          error = Boom.wrap(e, 422);
+        // TODO: we could get better at parse postgres error codes
+        } else {
+          // use a 502 error code since the issue is upstream with postgres, not
+          // this server
+          error = Boom.wrap(e, 502);
+        }
 
         // detail tends to be more specific information. So, if we have it, use.
         if (detail) {
@@ -20,8 +29,13 @@ export default (target, key, descriptor) => {
           error.reformat();
         }
 
-          reply(error);
+        // hint might provide useful information about how to fix the problem
+        if (hint) {
+          error.message += ` Hint: ${hint}`;
+          error.reformat();
         }
+
+        reply(error);
       } else if (!e.isBoom) {
         const { message } = e;
         let err;

src/utils.js

@@ -1,4 +1,5 @@
 import { omit, identity } from 'lodash';
+import { notImplemented } from 'boom';
 
 export const parseInclude = request => {
   const include = Array.isArray(request.query.include) ? request.query.include
@@ -8,7 +9,7 @@ export const parseInclude = request => {
   const noRequestModels = !request.models;
 
   if (noGetDb && noRequestModels) {
-    return new Error('`request.getDb` or `request.models` are not defined.'
+    return notImplemented('`request.getDb` or `request.models` are not defined.'
                    + 'Be sure to load hapi-sequelize before hapi-sequelize-crud.');
   }