thereadme/hapi-sequelize-crud
1
0
Fork 0
Code Issues 4 Pull Requests 1 Projects Releases Wiki Activity

Add feature to allow nested include and filtering relationships/associations #36

Open
labibramadhan wants to merge 8 commits from labibramadhan/include-with-filter into master
pull from: labibramadhan/include-with-filter
merge into: thereadme:master
Conversation 9 Commits 8 Files Changed 9 +327 -109
9 changed files with 327 additions and 109 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

59
README.md
View File

@ -109,10 +109,17 @@ Getting related models is easy, just use a query parameter `include`.
```js ```js
// returns all teams with their related City model // returns all teams with their related City model
// GET /teams?include=city // GET /teams?include=city or
// GET /teams?include={"model": "City"}
// results in a Sequelize query: // results in a Sequelize query:
Team.findAll({include: City}) Team.findAll({include: City})
or if association defined with an alias
// GET /players?include={"model": "Master", "as": "Couch"}
// results in a Sequelize query:
Players.findAll({include: Master, as: 'Couch'})
mdibaiee commented 2016-11-04 11:40:15 +00:00 (Migrated from github.com)
Review
Copy Link

Please get rid of the include key of the JSON passed to include, it's an unnecessary repetition.

It should look like this:
/teams?include={"model": "City", "where": { "name": "Healdsburg" }}

Please get rid of the `include` key of the JSON passed to `include`, it's an unnecessary repetition. It should look like this: `/teams?include={"model": "City", "where": { "name": "Healdsburg" }}`
labibramadhan commented 2016-11-07 02:03:04 +00:00 (Migrated from github.com)
Review
Copy Link

Fixed in my last commit, could you verify now?

Fixed in my last commit, could you verify now?
``` ```
If you want to get multiple related models, just pass multiple `include` parameters. If you want to get multiple related models, just pass multiple `include` parameters.
@ -133,6 +140,56 @@ For models that have a many-to-many relationship, you can also pass the plural v
Team.findAll({include: [Player]}) Team.findAll({include: [Player]})
``` ```
Filtering by related models property, you can pass **where** paremeter inside each **include** items object.
```js
// returns all team with their related City where City property name equals Healdsburg
// GET /teams?include={"model": "City", "where": {"name": "Healdsburg"}}
// results in a Sequelize query:
Team.findAll({include: {model: City, where: {name: 'Healdsburg'}}})
```
More complex example with nested include, association alias and association filtering.
```js
// returns all team with its players along with its couch of each player
// GET /cities?include[]={
// "model": "Team",
// "include": {
// "model": "Player",
// "where": {
// "name": "Pinot"
// },
// "include": {
// "model": "Master",
// "as": "Coach",
// "where": {
// "name": "Shifu"
// }
// }
// }
// }
// results in a Sequelize query:
City.findAll({
include: {
model: Team,
include: {
model: Player,
where: {
name: Pinot
},
include: {
model: Master,
as: 'Coach',
where: {
name: 'Shifu'
}
}
}
}
})
```
## `limit` and `offset` queries ## `limit` and `offset` queries
Restricting list (`GET`) and scope queries to a restricted count can be done by passing `limit=<number>` and/or `offset=<number>`. Restricting list (`GET`) and scope queries to a restricted count can be done by passing `limit=<number>` and/or `offset=<number>`.

42
src/associations/one-to-many.js
View File

@ -19,14 +19,14 @@ export default (server, a, b, names, options) => {
update(server, a, b, names); update(server, a, b, names);
}; };
export const get = (server, a, b, names) => { export const get = async (server, a, b, names) => {
server.route({ server.route({
method: 'GET', method: 'GET',
path: `${prefix}/${names.a.singular}/{aid}/${names.b.singular}/{bid}`, path: `${prefix}${names.a.singular}/{aid}/${names.b.singular}/{bid}`,
@error @error
async handler(request, reply) { async handler(request, reply) {
const include = parseInclude(request); const include = await parseInclude(request);
const base = await a.findOne({ const base = await a.findOne({
where: { where: {
@ -51,14 +51,14 @@ export const get = (server, a, b, names) => {
}); });
}; };
export const list = (server, a, b, names) => { export const list = async (server, a, b, names) => {
server.route({ server.route({
method: 'GET', method: 'GET',
path: `${prefix}/${names.a.singular}/{aid}/${names.b.plural}`, path: `${prefix}${names.a.singular}/{aid}/${names.b.plural}`,
@error @error
async handler(request, reply) { async handler(request, reply) {
const include = parseInclude(request); const include = await parseInclude(request);
const where = parseWhere(request); const where = parseWhere(request);
const base = await a.findOne({ const base = await a.findOne({
@ -77,16 +77,16 @@ export const list = (server, a, b, names) => {
}); });
}; };
export const scope = (server, a, b, names) => { export const scope = async (server, a, b, names) => {
const scopes = Object.keys(b.options.scopes); const scopes = Object.keys(b.options.scopes);
server.route({ server.route({
method: 'GET', method: 'GET',
path: `${prefix}/${names.a.singular}/{aid}/${names.b.plural}/{scope}`, path: `${prefix}${names.a.singular}/{aid}/${names.b.plural}/{scope}`,
@error @error
async handler(request, reply) { async handler(request, reply) {
const include = parseInclude(request); const include = await parseInclude(request);
const where = parseWhere(request); const where = parseWhere(request);
const base = await a.findOne({ const base = await a.findOne({
@ -116,7 +116,7 @@ export const scope = (server, a, b, names) => {
}); });
}; };
export const scopeScope = (server, a, b, names) => { export const scopeScope = async (server, a, b, names) => {
const scopes = { const scopes = {
a: Object.keys(a.options.scopes), a: Object.keys(a.options.scopes),
b: Object.keys(b.options.scopes), b: Object.keys(b.options.scopes),
@ -124,11 +124,11 @@ export const scopeScope = (server, a, b, names) => {
server.route({ server.route({
method: 'GET', method: 'GET',
path: `${prefix}/${names.a.plural}/{scopea}/${names.b.plural}/{scopeb}`, path: `${prefix}${names.a.plural}/{scopea}/${names.b.plural}/{scopeb}`,
@error @error
async handler(request, reply) { async handler(request, reply) {
const include = parseInclude(request); const include = await parseInclude(request);
const where = parseWhere(request); const where = parseWhere(request);
const list = await b.scope(request.params.scopeb).findAll({ const list = await b.scope(request.params.scopeb).findAll({
@ -152,14 +152,14 @@ export const scopeScope = (server, a, b, names) => {
}); });
}; };
export const destroy = (server, a, b, names) => { export const destroy = async (server, a, b, names) => {
server.route({ server.route({
method: 'DELETE', method: 'DELETE',
path: `${prefix}/${names.a.singular}/{aid}/${names.b.plural}`, path: `${prefix}${names.a.singular}/{aid}/${names.b.plural}`,
@error @error
async handler(request, reply) { async handler(request, reply) {
const include = parseInclude(request); const include = await parseInclude(request);
const where = parseWhere(request); const where = parseWhere(request);
const base = await a.findOne({ const base = await a.findOne({
@ -179,16 +179,16 @@ export const destroy = (server, a, b, names) => {
}); });
}; };
export const destroyScope = (server, a, b, names) => { export const destroyScope = async (server, a, b, names) => {
const scopes = Object.keys(b.options.scopes); const scopes = Object.keys(b.options.scopes);
server.route({ server.route({
method: 'DELETE', method: 'DELETE',
path: `${prefix}/${names.a.singular}/{aid}/${names.b.plural}/{scope}`, path: `${prefix}${names.a.singular}/{aid}/${names.b.plural}/{scope}`,
@error @error
async handler(request, reply) { async handler(request, reply) {
const include = parseInclude(request); const include = await parseInclude(request);
const where = parseWhere(request); const where = parseWhere(request);
const base = await a.findOne({ const base = await a.findOne({
@ -221,14 +221,14 @@ export const destroyScope = (server, a, b, names) => {
}); });
}; };
export const update = (server, a, b, names) => { export const update = async (server, a, b, names) => {
server.route({ server.route({
method: 'PUT', method: 'PUT',
path: `${prefix}/${names.a.singular}/{aid}/${names.b.plural}`, path: `${prefix}${names.a.singular}/{aid}/${names.b.plural}`,
@error @error
async handler(request, reply) { async handler(request, reply) {
const include = parseInclude(request); const include = await parseInclude(request);
const where = parseWhere(request); const where = parseWhere(request);
const base = await a.findOne({ const base = await a.findOne({

18
src/associations/one-to-one.js
View File

@ -14,14 +14,14 @@ export default (server, a, b, names, options) => {
update(server, a, b, names); update(server, a, b, names);
}; };
export const get = (server, a, b, names) => { export const get = async (server, a, b, names) => {
server.route({ server.route({
method: 'GET', method: 'GET',
path: `${prefix}/${names.a.singular}/{aid}/${names.b.singular}`, path: `${prefix}${names.a.singular}/{aid}/${names.b.singular}`,
@error @error
async handler(request, reply) { async handler(request, reply) {
const include = parseInclude(request); const include = await parseInclude(request);
const where = parseWhere(request); const where = parseWhere(request);
const base = await a.findOne({ const base = await a.findOne({
@ -47,7 +47,7 @@ export const get = (server, a, b, names) => {
export const create = (server, a, b, names) => { export const create = (server, a, b, names) => {
server.route({ server.route({
method: 'POST', method: 'POST',
path: `${prefix}/${names.a.singular}/{id}/${names.b.singular}`, path: `${prefix}${names.a.singular}/{id}/${names.b.singular}`,
@error @error
async handler(request, reply) { async handler(request, reply) {
@ -67,14 +67,14 @@ export const create = (server, a, b, names) => {
}); });
}; };
export const destroy = (server, a, b, names) => { export const destroy = async (server, a, b, names) => {
server.route({ server.route({
method: 'DELETE', method: 'DELETE',
path: `${prefix}/${names.a.singular}/{aid}/${names.b.singular}/{bid}`, path: `${prefix}${names.a.singular}/{aid}/${names.b.singular}/{bid}`,
@error @error
async handler(request, reply) { async handler(request, reply) {
const include = parseInclude(request); const include = await parseInclude(request);
const where = parseWhere(request); const where = parseWhere(request);
const base = await a.findOne({ const base = await a.findOne({
@ -99,11 +99,11 @@ export const destroy = (server, a, b, names) => {
export const update = (server, a, b, names) => { export const update = (server, a, b, names) => {
server.route({ server.route({
method: 'PUT', method: 'PUT',
path: `${prefix}/${names.a.singular}/{aid}/${names.b.singular}/{bid}`, path: `${prefix}${names.a.singular}/{aid}/${names.b.singular}/{bid}`,
@error @error
async handler(request, reply) { async handler(request, reply) {
const include = parseInclude(request); const include = await parseInclude(request);
const where = parseWhere(request); const where = parseWhere(request);
const base = await a.findOne({ const base = await a.findOne({

87
src/crud-include.integration.test.js
View File

@ -6,7 +6,7 @@ const STATUS_OK = 200;
setup(test); setup(test);
test('belongsTo /team?include=city', async (t) => { test('belongsTo /team?include=city', async(t) => {
const { server, instances } = t.context; const { server, instances } = t.context;
joeybaker commented 2016-11-09 23:37:15 +00:00 (Migrated from github.com)
Review
Copy Link

Why remove all these spaces?

Why remove all these spaces?
labibramadhan commented 2016-11-10 02:10:42 +00:00 (Migrated from github.com)
Review
Copy Link

sorry this is from my IDE settings, but i don't see any eslint error

sorry this is from my IDE settings, but i don't see any eslint error
const { team1, city1 } = instances; const { team1, city1 } = instances;
const path = `/team/${team1.id}?include=city`; const path = `/team/${team1.id}?include=city`;
@ -17,7 +17,7 @@ test('belongsTo /team?include=city', async (t) => {
t.is(result.City.id, city1.id); t.is(result.City.id, city1.id);
}); });
test('belongsTo /team?include=cities', async (t) => { test('belongsTo /team?include=cities', async(t) => {
const { server, instances } = t.context; const { server, instances } = t.context;
const { team1, city1 } = instances; const { team1, city1 } = instances;
const path = `/team/${team1.id}?include=cities`; const path = `/team/${team1.id}?include=cities`;
@ -28,7 +28,7 @@ test('belongsTo /team?include=cities', async (t) => {
t.is(result.City.id, city1.id); t.is(result.City.id, city1.id);
}); });
test('hasMany /team?include=player', async (t) => { test('hasMany /team?include=player', async(t) => {
const { server, instances } = t.context; const { server, instances } = t.context;
const { team1, player1, player2 } = instances; const { team1, player1, player2 } = instances;
const path = `/team/${team1.id}?include=player`; const path = `/team/${team1.id}?include=player`;
@ -42,7 +42,7 @@ test('hasMany /team?include=player', async (t) => {
t.truthy(playerIds.includes(player2.id)); t.truthy(playerIds.includes(player2.id));
}); });
test('hasMany /team?include=players', async (t) => { test('hasMany /team?include=players', async(t) => {
const { server, instances } = t.context; const { server, instances } = t.context;
const { team1, player1, player2 } = instances; const { team1, player1, player2 } = instances;
const path = `/team/${team1.id}?include=players`; const path = `/team/${team1.id}?include=players`;
@ -56,7 +56,18 @@ test('hasMany /team?include=players', async (t) => {
t.truthy(playerIds.includes(player2.id)); t.truthy(playerIds.includes(player2.id));
}); });
test('multiple includes /team?include=players&include=city', async (t) => { test('belongsTo with alias /player?include={"model": "Master", "as": "Coach"}', async(t) => {
const { server, instances } = t.context;
const { team1, master1 } = instances;
const path = `/player/${team1.id}?include={"model": "Master", "as": "Coach"}`;
const { result, statusCode } = await server.inject(path);
t.is(statusCode, STATUS_OK);
t.is(result.id, team1.id);
t.is(result.Coach.id, master1.id);
});
test('multiple includes /team?include=players&include=city', async(t) => {
const { server, instances } = t.context; const { server, instances } = t.context;
const { team1, player1, player2, city1 } = instances; const { team1, player1, player2, city1 } = instances;
const path = `/team/${team1.id}?include=players&include=city`; const path = `/team/${team1.id}?include=players&include=city`;
@ -71,7 +82,7 @@ test('multiple includes /team?include=players&include=city', async (t) => {
t.is(result.City.id, city1.id); t.is(result.City.id, city1.id);
}); });
test('multiple includes /team?include[]=players&include[]=city', async (t) => { test('multiple includes /team?include[]=players&include[]=city', async(t) => {
const { server, instances } = t.context; const { server, instances } = t.context;
const { team1, player1, player2, city1 } = instances; const { team1, player1, player2, city1 } = instances;
const path = `/team/${team1.id}?include[]=players&include[]=city`; const path = `/team/${team1.id}?include[]=players&include[]=city`;
@ -85,3 +96,67 @@ test('multiple includes /team?include[]=players&include[]=city', async (t) => {
t.truthy(playerIds.includes(player2.id)); t.truthy(playerIds.includes(player2.id));
t.is(result.City.id, city1.id); t.is(result.City.id, city1.id);
}); });
test('multiple includes /team?include[]=players&include[]={"model": "City"}', async(t) => {
const { server, instances } = t.context;
const { team1, player1, player2, city1 } = instances;
const path = `/team/${team1.id}?include[]=players&include[]={"model": "City"}`;
const { result, statusCode } = await server.inject(path);
t.is(statusCode, STATUS_OK);
t.is(result.id, team1.id);
const playerIds = result.Players.map(({ id }) => id);
t.truthy(playerIds.includes(player1.id));
t.truthy(playerIds.includes(player2.id));
t.is(result.City.id, city1.id);
});
test('include filter /teams?include[]={"model": "City", "where": {"name": "Healdsburg"}}'
, async(t) => {
const { server } = t.context;
const url = '/teams?include[]={"model": "City", "where": {"name": "Healdsburg"}}';
const method = 'GET';
const { statusCode } = await server.inject({ url, method });
t.is(statusCode, STATUS_OK);
});
test('nested include filter ' +
'/citiy?include[]=' +
'{"model": "Team", "include": {"model": "City", "where": {"name": "Healdsburg"}}}'
, async(t) => {
const { instances, server } = t.context;
const { city1, team1, team2 } = instances;
const url = '/city?include[]=' +
'{"model": "Team", "include": {"model": "City", "where": {"name": "Healdsburg"}}}';
const method = 'GET';
const { result, statusCode } = await server.inject({ url, method });
t.is(statusCode, STATUS_OK);
t.is(result.id, city1.id);
const teamIds = result.Teams.map(({ id }) => id);
t.truthy(teamIds.includes(team1.id));
t.truthy(teamIds.includes(team2.id));
});
test('complex include ' +
'/cities?include[]={"model":"Team", ' +
'"include":{ "model":"Player", "where":{"name": "Pinot"}, ' +
'"include":{ "model":"Master", "as":"Coach", "where":{"name": "Shifu"}}}}'
, async(t) => {
const { instances, server } = t.context;
const { city1, master1, player2, team1 } = instances;
const method = 'GET';
const url = '/cities?include[]={"model":"Team", ' +
'"include":{ "model":"Player", "where":{"name": "Pinot"}, ' +
'"include":{ "model":"Master", "as":"Coach", "where":{"name": "Shifu"}}}}';
const { result, statusCode } = await server.inject({ url, method });
t.is(statusCode, STATUS_OK);
t.is(result[0].id, city1.id);
t.is(result[0].Teams[0].id, team1.id);
t.is(result[0].Teams[0].Players[0].id, player2.id);
t.is(result[0].Teams[0].Players[0].Coach.id, master1.id);
});

102
src/crud.js
View File

@ -5,16 +5,16 @@ import _ from 'lodash';
import { parseInclude, parseWhere, parseLimitAndOffset, parseOrder } from './utils'; import { parseInclude, parseWhere, parseLimitAndOffset, parseOrder } from './utils';
import { notFound } from 'boom'; import { notFound } from 'boom';
import * as associations from './associations/index'; import * as associations from './associations/index';
import getConfigForMethod from './get-config-for-method.js'; import getConfigForMethod, { sequelizeOperators } from './get-config-for-method.js';
const createAll = ({ const createAll = ({
server, server,
model, model,
prefix, prefix,
config, config,
attributeValidation, attributeValidation,
associationValidation, associationValidation,
scopes, scopes,
}) => { }) => {
Object.keys(methods).forEach((method) => { Object.keys(methods).forEach((method) => {
methods[method]({ methods[method]({
@ -35,27 +35,28 @@ const createAll = ({
export { associations }; export { associations };
/* /*
The `models` option, becomes `permissions`, and can look like: The `models` option, becomes `permissions`, and can look like:
``` ```
models: ['cat', 'dog'] models: ['cat', 'dog']
``` ```
or or
``` ```
models: { models: {
cat: ['list', 'get'] cat: ['list', 'get']
, dog: true // all , dog: true // all
} }
``` ```
*/ */
export default (server, model, { prefix, defaultConfig: config, models: permissions }) => { export default (server, model, { prefix, defaultConfig: config, models: permissions }) => {
const modelName = model._singular; const modelName = model._singular;
const modelAttributes = Object.keys(model.attributes); const modelAttributes = Object.keys(model.attributes);
const associatedModelNames = Object.keys(model.associations); const associatedModelNames = Object.keys(model.associations);
const associatedModelAliases = _.map(model.associations, (assoc => assoc.as));
const modelAssociations = [ const modelAssociations = [
...associatedModelNames, ...associatedModelNames,
..._.flatMap(associatedModelNames, (associationName) => { ..._.flatMap(associatedModelNames, (associationName) => {
@ -71,13 +72,28 @@ export default (server, model, { prefix, defaultConfig: config, models: permissi
return params; return params;
}, {}); }, {});
const validAssociations = modelAssociations.length const modelsHasAssociations = modelAssociations && modelAssociations.length;
? joi.string().valid(...modelAssociations) const validAssociationsString = modelsHasAssociations
: joi.valid(null); ? joi.string().valid(...modelAssociations)
: joi.valid(null);
const validAssociationsObject = modelsHasAssociations
? joi.object().keys({
model: joi.string().valid(...modelAssociations),
where: joi.object().keys({
...attributeValidation,
...sequelizeOperators,
}),
as: joi.string().valid(...associatedModelAliases),
include: joi.any(), // @Todo: should validate the same as associationValidation var below
})
: joi.valid(null);
const associationValidation = { const associationValidation = {
include: [joi.array().items(validAssociations), validAssociations], include: [
joi.array().items(validAssociationsString, validAssociationsObject),
validAssociationsString,
validAssociationsObject,
],
}; };
const scopes = Object.keys(model.options.scopes); const scopes = Object.keys(model.options.scopes);
// if we don't have any permissions set, just create all the methods // if we don't have any permissions set, just create all the methods
@ -91,11 +107,11 @@ export default (server, model, { prefix, defaultConfig: config, models: permissi
associationValidation, associationValidation,
scopes, scopes,
}); });
// if permissions are set, but we can't parse them, throw an error // if permissions are set, but we can't parse them, throw an error
} else if (!Array.isArray(permissions)) { } else if (!Array.isArray(permissions)) {
throw new Error('hapi-sequelize-crud: `models` property must be an array'); 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 // 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 // are no permissions to be set, so just create all methods and move on
} else if (permissions.includes(modelName)) { } else if (permissions.includes(modelName)) {
createAll({ createAll({
server, server,
@ -106,7 +122,7 @@ export default (server, model, { prefix, defaultConfig: config, models: permissi
associationValidation, associationValidation,
scopes, scopes,
}); });
// if we've gotten here, we have complex permissions and need to set them // if we've gotten here, we have complex permissions and need to set them
} else { } else {
const permissionOptions = permissions.filter((permission) => { const permissionOptions = permissions.filter((permission) => {
return permission.model === modelName; return permission.model === modelName;
@ -147,14 +163,14 @@ export default (server, model, { prefix, defaultConfig: config, models: permissi
} }
}; };
export const list = ({ server, model, prefix = '/', config }) => { export const list = async ({ server, model, prefix = '/', config }) => {
server.route({ server.route({
method: 'GET', method: 'GET',
path: path.join(prefix, model._plural), path: path.join(prefix, model._plural),
@error @error
async handler(request, reply) { async handler(request, reply) {
const include = parseInclude(request); const include = await parseInclude(request);
const where = parseWhere(request); const where = parseWhere(request);
const { limit, offset } = parseLimitAndOffset(request); const { limit, offset } = parseLimitAndOffset(request);
const order = parseOrder(request); const order = parseOrder(request);
@ -174,14 +190,14 @@ export const list = ({ server, model, prefix = '/', config }) => {
}); });
}; };
export const get = ({ server, model, prefix = '/', config }) => { export const get = async ({ server, model, prefix = '/', config }) => {
server.route({ server.route({
method: 'GET', method: 'GET',
path: path.join(prefix, model._singular, '{id?}'), path: path.join(prefix, model._singular, '{id?}'),
@error @error
async handler(request, reply) { async handler(request, reply) {
const include = parseInclude(request); const include = await parseInclude(request);
const where = parseWhere(request); const where = parseWhere(request);
const { id } = request.params; const { id } = request.params;
if (id) where[model.primaryKeyField] = id; if (id) where[model.primaryKeyField] = id;
@ -198,14 +214,14 @@ export const get = ({ server, model, prefix = '/', config }) => {
}); });
}; };
export const scope = ({ server, model, prefix = '/', config }) => { export const scope = async ({ server, model, prefix = '/', config }) => {
server.route({ server.route({
method: 'GET', method: 'GET',
path: path.join(prefix, model._plural, '{scope}'), path: path.join(prefix, model._plural, '{scope}'),
@error @error
async handler(request, reply) { async handler(request, reply) {
const include = parseInclude(request); const include = await parseInclude(request);
const where = parseWhere(request); const where = parseWhere(request);
const { limit, offset } = parseLimitAndOffset(request); const { limit, offset } = parseLimitAndOffset(request);
const order = parseOrder(request); const order = parseOrder(request);
@ -255,9 +271,9 @@ export const destroy = ({ server, model, prefix = '/', config }) => {
if (!list.length) { if (!list.length) {
return void reply(id return void reply(id
? notFound(`${id} not found.`) ? notFound(`${id} not found.`)
: notFound('Nothing found.') : notFound('Nothing found.')
); );
} }
await Promise.all(list.map(instance => instance.destroy())); await Promise.all(list.map(instance => instance.destroy()));
@ -284,9 +300,9 @@ export const destroyAll = ({ server, model, prefix = '/', config }) => {
if (!list.length) { if (!list.length) {
return void reply(id return void reply(id
? notFound(`${id} not found.`) ? notFound(`${id} not found.`)
: notFound('Nothing found.') : notFound('Nothing found.')
); );
} }
await Promise.all(list.map(instance => instance.destroy())); await Promise.all(list.map(instance => instance.destroy()));
@ -299,14 +315,14 @@ export const destroyAll = ({ server, model, prefix = '/', config }) => {
}); });
}; };
export const destroyScope = ({ server, model, prefix = '/', config }) => { export const destroyScope = async ({ server, model, prefix = '/', config }) => {
server.route({ server.route({
method: 'DELETE', method: 'DELETE',
path: path.join(prefix, model._plural, '{scope}'), path: path.join(prefix, model._plural, '{scope}'),
@error @error
async handler(request, reply) { async handler(request, reply) {
const include = parseInclude(request); const include = await parseInclude(request);
const where = parseWhere(request); const where = parseWhere(request);
if (include instanceof Error) return void reply(include); if (include instanceof Error) return void reply(include);

73
src/utils.js
View File

@ -1,5 +1,7 @@
import { omit, identity, toNumber, isString, isUndefined } from 'lodash'; import { omit, identity, toNumber, isString, isUndefined } from 'lodash';
import { notImplemented } from 'boom'; import { notImplemented } from 'boom';
import joi from 'joi';
import Promise from 'bluebird';
const sequelizeKeys = ['include', 'order', 'limit', 'offset']; const sequelizeKeys = ['include', 'order', 'limit', 'offset'];
@ -8,7 +10,7 @@ const getModels = (request) => {
const noRequestModels = !request.models; const noRequestModels = !request.models;
if (noGetDb && noRequestModels) { if (noGetDb && noRequestModels) {
return notImplemented('`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.'); + 'Be sure to load hapi-sequelize before hapi-sequelize-crud.');
} }
const { models } = noGetDb ? request : request.getDb(); const { models } = noGetDb ? request : request.getDb();
@ -16,31 +18,68 @@ const getModels = (request) => {
return models; return models;
}; };
export const parseInclude = request => { const getModelInstance = (models, includeItem) => {
return new Promise(async(resolve) => {
if (includeItem) {
if (typeof includeItem !== 'object') {
joeybaker commented 2016-11-04 16:21:28 +00:00 (Migrated from github.com)
Review
Copy Link

JSON.parse will throw if not handed valid JSON. We should use a try/catch here.

`JSON.parse` will throw if not handed valid JSON. We should use a try/catch here.
labibramadhan commented 2016-11-07 01:56:57 +00:00 (Migrated from github.com)
Review
Copy Link

what code inside the catch do you prefer?

what code inside the catch do you prefer?
joeybaker commented 2016-11-07 17:48:30 +00:00 (Migrated from github.com)
Review
Copy Link

5ba9d7d261/src/utils.js (L50-L54) and 5ba9d7d261/src/utils.js (L78-L83) are both options. Thanks!

https://github.com/mdibaiee/hapi-sequelize-crud/blob/5ba9d7d26100f1a1a82367097342a8c35d43a26f/src/utils.js#L50-L54 and https://github.com/mdibaiee/hapi-sequelize-crud/blob/5ba9d7d26100f1a1a82367097342a8c35d43a26f/src/utils.js#L78-L83 are both options. Thanks!
const singluarOrPluralMatch = Object.keys(models).find((modelName) => {
labibramadhan commented 2016-11-08 01:29:34 +00:00 (Migrated from github.com)
Review
Copy Link

maybe like this @joeybaker?

maybe like this @joeybaker?
joeybaker commented 2016-11-08 17:16:04 +00:00 (Migrated from github.com)
Review
Copy Link

Kinda! The try/catch part is right, but the joi check should be a part of the validation. What you have here will always pass the if condition, because joi.string() always returns a truthy value.

What do you think of something like this in crud.js on line 77?

  const assocationJSONvaliation = joi.string().regex(/^\{.*?"model":.*?\}$/);
  const associationValidation = {
    include: [
      joi.array().items(validAssociations),
      joi.array().items(assocationJSONvaliation),
      validAssociations,
      assocationJSONvaliation,
    ],
  };
Kinda! The try/catch part is right, but the joi check should be a part of the validation. What you have here will always pass the `if` condition, because `joi.string()` always returns a truthy value. What do you think of something like this in `crud.js` on line 77? ``` js const assocationJSONvaliation = joi.string().regex(/^\{.*?"model":.*?\}$/); const associationValidation = { include: [ joi.array().items(validAssociations), joi.array().items(assocationJSONvaliation), validAssociations, assocationJSONvaliation, ], }; ```
labibramadhan commented 2016-11-09 01:48:34 +00:00 (Migrated from github.com)
Review
Copy Link

but with this code, it won't validate the "model" name and the "where" object isn't it?
if the JSON string doesn't parse good so it caught as a string, but later it won't pass because crud.js that i pushed has something like joi.string().valid(...modelAssociations)

but with this code, it won't validate the "**model**" name and the "**where**" object isn't it? if the JSON string doesn't parse good so it caught as a string, but later it won't pass because crud.js that i pushed has something like `joi.string().valid(...modelAssociations)`
labibramadhan commented 2016-11-09 01:52:20 +00:00 (Migrated from github.com)
Review
Copy Link

currently my version of crud.js has this to validate its association object:

joi.object().keys({
        model: joi.string().valid(...modelAssociations),
        where: joi.object().keys({
          ...attributeValidation,
          ...sequelizeOperators,
        }),
      })
currently my version of crud.js has this to validate its association object: ``` joi.object().keys({ model: joi.string().valid(...modelAssociations), where: joi.object().keys({ ...attributeValidation, ...sequelizeOperators, }), }) ```
joeybaker commented 2016-11-09 05:46:48 +00:00 (Migrated from github.com)
Review
Copy Link

I get it. I was opting for: "if you pass JSON, we're going to skip validating it … it's too complex to do correctly".

Here's the problem I see with:

joi.object().keys({
        model: joi.string().valid(...modelAssociations),
        where: joi.object().keys({
          ...attributeValidation,
          ...sequelizeOperators,
        }),
      })

It ensures you get a valid JS object to validate. But you won't. You'll get a string that contains JSON. That's why I was opting for the regex of a string. Did I get confused there?

I get it. I was opting for: "if you pass JSON, we're going to skip validating it … it's too complex to do correctly". Here's the problem I see with: ``` js joi.object().keys({ model: joi.string().valid(...modelAssociations), where: joi.object().keys({ ...attributeValidation, ...sequelizeOperators, }), }) ``` It ensures you get a valid JS object to validate. But you won't. You'll get a string that contains JSON. That's why I was opting for the regex of a string. Did I get confused there?
labibramadhan commented 2016-11-09 19:01:15 +00:00 (Migrated from github.com)
Review
Copy Link

sorry @joeybaker, but i still didn't get it, what the main purpose of adding the json validation you want? because i have tried several cases of using include parameter and i didn't find any include parameter validation problem

sorry @joeybaker, but i still didn't get it, what the main purpose of adding the json validation you want? because i have tried several cases of using include parameter and i didn't find any include parameter validation problem
const { _singular, _plural } = models[modelName];
return _singular === includeItem || _plural === includeItem;
});
if (singluarOrPluralMatch) {
return resolve(models[singluarOrPluralMatch]);
}
}
if (typeof includeItem === 'string' && models.hasOwnProperty(includeItem)) {
return resolve(models[includeItem]);
} else if (typeof includeItem === 'object') {
if (
typeof includeItem.model === 'string' &&
includeItem.model.length &&
models.hasOwnProperty(includeItem.model)
) {
includeItem.model = models[includeItem.model];
}
if (includeItem.hasOwnProperty('include')) {
includeItem.include = await getModelInstance(models, includeItem.include);
return resolve(includeItem);
} else {
return resolve(includeItem);
}
}
}
return resolve(includeItem);
});
};
export const parseInclude = async(request) => {
if (typeof request.query.include === 'undefined') return [];
const include = Array.isArray(request.query.include) const include = Array.isArray(request.query.include)
? request.query.include ? request.query.include
: [request.query.include] : [request.query.include]
; ;
const models = getModels(request); const models = getModels(request);
if (models.isBoom) return models; if (models.isBoom) return models;
return include.map(a => { const jsonValidation = joi.string().regex(/^\{.*?"model":.*?\}$/);
const singluarOrPluralMatch = Object.keys(models).find((modelName) => { const includes = include.map(async(b) => {
const { _singular, _plural } = models[modelName]; let a = b;
return _singular === a || _plural === a; try {
}); if (!jsonValidation.validate(a).error) {
a = JSON.parse(b);
if (singluarOrPluralMatch) return models[singluarOrPluralMatch]; }
} catch (e) {
if (typeof a === 'string') return models[a]; //
if (a && typeof a.model === 'string' && a.model.length) {
a.model = models[a.model];
} }
return a; return getModelInstance(models, a);
}).filter(identity); }).filter(identity);
return await Promise.all(includes);
}; };
export const parseWhere = request => { export const parseWhere = request => {

18
test/fixtures/models/master.js vendored Normal file
View File

@ -0,0 +1,18 @@
export default (sequelize, DataTypes) => {
return sequelize.define('Master', {
id: {
type: DataTypes.INTEGER,
primaryKey: true,
autoIncrement: true,
},
name: DataTypes.STRING,
}, {
classMethods: {
associate: (models) => {
models.Master.hasMany(models.Player, {
foreignKey: 'coachId'
});
},
},
});
};

4
test/fixtures/models/player.js vendored
View File

@ -14,6 +14,10 @@ export default (sequelize, DataTypes) => {
models.Player.belongsTo(models.Team, { models.Player.belongsTo(models.Team, {
foreignKey: { name: 'teamId' }, foreignKey: { name: 'teamId' },
}); });
models.Player.belongsTo(models.Master, {
foreignKey: 'coachId',
as: 'Coach',
labibramadhan commented 2016-11-09 18:52:09 +00:00 (Migrated from github.com)
Review
Copy Link

needed for association/relationship alias test

needed for association/relationship alias test
joeybaker commented 2016-11-09 23:36:02 +00:00 (Migrated from github.com)
Review
Copy Link

Can you not use Teams and Players? Just so we don't get an overly-complex mock schema?

Can you not use Teams and Players? Just so we don't get an overly-complex mock schema?
joeybaker commented 2016-11-10 01:22:07 +00:00 (Migrated from github.com)
Review
Copy Link

Ah! I see. Makes sense.

Ah! I see. Makes sense.
labibramadhan commented 2016-11-10 02:10:08 +00:00 (Migrated from github.com)
Review
Copy Link

is it ok then?

is it ok then?
joeybaker commented 2016-11-10 19:25:44 +00:00 (Migrated from github.com)
Review
Copy Link

Yup!

Yup!
});
}, },
}, },
scopes: { scopes: {

33
test/integration-setup.js
View File

@ -14,15 +14,16 @@ const modelNames = [
{ Singluar: 'City', singular: 'city', Plural: 'Cities', plural: 'cities' }, { Singluar: 'City', singular: 'city', Plural: 'Cities', plural: 'cities' },
{ Singluar: 'Team', singular: 'team', Plural: 'Teams', plural: 'teams' }, { Singluar: 'Team', singular: 'team', Plural: 'Teams', plural: 'teams' },
{ Singluar: 'Player', singular: 'player', Plural: 'Players', plural: 'players' }, { Singluar: 'Player', singular: 'player', Plural: 'Players', plural: 'players' },
{ Singluar: 'Master', singular: 'master', Plural: 'Masters', plural: 'masters' },
]; ];
export default (test) => { export default (test) => {
test.beforeEach('get an open port', async (t) => { test.beforeEach('get an open port', async(t) => {
t.context.port = await getPort(); t.context.port = await getPort();
}); });
test.beforeEach('setup server', async (t) => { test.beforeEach('setup server', async(t) => {
const sequelize = t.context.sequelize = new Sequelize({ const sequelize = t.context.sequelize = new Sequelize({
dialect: 'sqlite', dialect: 'sqlite',
logging: false, logging: false,
@ -46,25 +47,33 @@ export default (test) => {
}); });
await server.register({ await server.register({
register: require('../src/index.js'), register: require('../src/index.js'),
options: { options: {
name: dbName, name: dbName,
},
}, },
},
); );
}); });
test.beforeEach('create data', async (t) => { test.beforeEach('create data', async(t) => {
const { Player, Team, City } = t.context.sequelize.models; const { Player, Master, Team, City } = t.context.sequelize.models;
const city1 = await City.create({ name: 'Healdsburg' }); const city1 = await City.create({ name: 'Healdsburg' });
const team1 = await Team.create({ name: 'Baseballs', cityId: city1.id }); const team1 = await Team.create({ name: 'Baseballs', cityId: city1.id });
const team2 = await Team.create({ name: 'Footballs', cityId: city1.id }); const team2 = await Team.create({ name: 'Footballs', cityId: city1.id });
const master1 = await Master.create({ name: 'Shifu' });
const master2 = await Master.create({ name: 'Oogway' });
const player1 = await Player.create({ const player1 = await Player.create({
name: 'Cat', teamId: team1.id, active: true, name: 'Cat', teamId: team1.id, active: true, coachId: master1.id
}); });
const player2 = await Player.create({ name: 'Pinot', teamId: team1.id }); const player2 = await Player.create({
const player3 = await Player.create({ name: 'Syrah', teamId: team2.id }); name: 'Pinot', teamId: team1.id, coachId: master1.id
t.context.instances = { city1, team1, team2, player1, player2, player3 }; });
const player3 = await Player.create({
name: 'Syrah', teamId: team2.id, coachId: master2.id
});
t.context.instances = {
city1, team1, team2, player1, player2, player3, master1, master2
};
}); });
// kill the server so that we can exit and don't leak memory // kill the server so that we can exit and don't leak memory