From 867c6e28a17f5ff079b388cde92837aac4fe2f98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gabriel=20Th=C3=A9ron?= Date: Tue, 17 Oct 2017 11:32:29 +0200 Subject: [PATCH 1/5] Adds a check for criteria's symbols in list search --- lib/Controllers/list.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/Controllers/list.js b/lib/Controllers/list.js index 5bd435f..588102a 100644 --- a/lib/Controllers/list.js +++ b/lib/Controllers/list.js @@ -98,8 +98,8 @@ List.prototype.fetch = function(req, res, context) { item[attr] = query; search.push(item); }); - - if (Object.keys(criteria).length) + + if (Object.keys(criteria).length || (Object.getOwnPropertySymbols && Object.getOwnPropertySymbols(criteria).length)) criteria = Sequelize.and(criteria, Sequelize.or.apply(null, search)); else criteria = Sequelize.or.apply(null, search); @@ -142,7 +142,7 @@ List.prototype.fetch = function(req, res, context) { criteria = _.assign(criteria, extraSearchCriteria); // do the actual lookup - if (Object.keys(criteria).length) + if (Object.keys(criteria).length || (Object.getOwnPropertySymbols && Object.getOwnPropertySymbols(criteria).length)) options.where = criteria; if (req.query.scope) { From 46fcdd3b1c87aaee5a41bd1b64a85c544efe304f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gabriel=20Th=C3=A9ron?= Date: Wed, 18 Oct 2017 12:49:17 +0200 Subject: [PATCH 2/5] Replaces Object.keys() with getKeys() mixing strings and symbols --- lib/Controllers/list.js | 13 +++++++------ lib/Controllers/read.js | 7 ++++--- lib/Controllers/update.js | 5 +++-- lib/Resource.js | 15 ++++++++------- lib/util.js | 10 ++++++++++ tests/util.test.js | 27 +++++++++++++++++++++++++++ 6 files changed, 59 insertions(+), 18 deletions(-) create mode 100644 lib/util.js create mode 100644 tests/util.test.js diff --git a/lib/Controllers/list.js b/lib/Controllers/list.js index 588102a..e181226 100644 --- a/lib/Controllers/list.js +++ b/lib/Controllers/list.js @@ -3,7 +3,8 @@ var util = require('util'), Base = require('./base'), _ = require('lodash'), - errors = require('../Errors'); + errors = require('../Errors'), + getKeys = require('../util').keys; var List = function(args) { List.super_.call(this, args); @@ -73,7 +74,7 @@ List.prototype.fetch = function(req, res, context) { var search = []; var searchOperator = searchData.operator || '$like'; var searchAttributes = - searchData.attributes || Object.keys(model.rawAttributes); + searchData.attributes || getKeys(model.rawAttributes); searchAttributes.forEach(function(attr) { if(stringOperators.test(searchOperator)){ var attrType = model.rawAttributes[attr].type; @@ -99,7 +100,7 @@ List.prototype.fetch = function(req, res, context) { search.push(item); }); - if (Object.keys(criteria).length || (Object.getOwnPropertySymbols && Object.getOwnPropertySymbols(criteria).length)) + if (getKeys(criteria).length) criteria = Sequelize.and(criteria, Sequelize.or.apply(null, search)); else criteria = Sequelize.or.apply(null, search); @@ -122,7 +123,7 @@ List.prototype.fetch = function(req, res, context) { order.push([sortColumn, 'ASC']); } }); - var allowedColumns = this.resource.sort.attributes || Object.keys(model.rawAttributes); + var allowedColumns = this.resource.sort.attributes || getKeys(model.rawAttributes); var disallowedColumns = _.difference(columnNames, allowedColumns); if (disallowedColumns.length) { throw new errors.BadRequestError('Sorting not allowed on given attributes', disallowedColumns); @@ -138,11 +139,11 @@ List.prototype.fetch = function(req, res, context) { return result; }, {}); - if (Object.keys(extraSearchCriteria).length) + if (getKeys(extraSearchCriteria).length) criteria = _.assign(criteria, extraSearchCriteria); // do the actual lookup - if (Object.keys(criteria).length || (Object.getOwnPropertySymbols && Object.getOwnPropertySymbols(criteria).length)) + if (getKeys(criteria).length) options.where = criteria; if (req.query.scope) { diff --git a/lib/Controllers/read.js b/lib/Controllers/read.js index 3f4dc1a..4b468f5 100644 --- a/lib/Controllers/read.js +++ b/lib/Controllers/read.js @@ -2,7 +2,8 @@ var util = require('util'), Base = require('./base'), - errors = require('../Errors'); + errors = require('../Errors'), + getKeys = require('../util').keys; var Read = function(args) { Read.super_.call(this, args); @@ -26,12 +27,12 @@ Read.prototype.fetch = function(req, res, context) { options.attributes = options.attributes || this.resource.attributes; // remove params that are already accounted for in criteria - Object.keys(criteria).forEach(function(attr) { delete req.params[attr]; }); + getKeys(criteria).forEach(function(attr) { delete req.params[attr]; }); endpoint.attributes.forEach(function(attribute) { if (attribute in req.params) criteria[attribute] = req.params[attribute]; }); - if (Object.keys(criteria).length) { + if (getKeys(criteria).length) { options.where = criteria; } diff --git a/lib/Controllers/update.js b/lib/Controllers/update.js index 14db168..323da82 100644 --- a/lib/Controllers/update.js +++ b/lib/Controllers/update.js @@ -3,7 +3,8 @@ var _ = require('lodash'), util = require('util'), Base = require('./base'), - ReadController = require('./read'); + ReadController = require('./read'), + getKeys = require('../util').keys; var Update = function(args) { if (args.resource.updateMethod) @@ -49,7 +50,7 @@ Update.prototype.write = function(req, res, context) { // check if reload is needed var reloadAfter = self.resource.reloadInstances && - Object.keys(self.resource.associationsInfo).some(function(attr) { + getKeys(self.resource.associationsInfo).some(function(attr) { return instance._changed.hasOwnProperty(attr); }); diff --git a/lib/Resource.js b/lib/Resource.js index 2e95689..78633eb 100644 --- a/lib/Resource.js +++ b/lib/Resource.js @@ -5,7 +5,8 @@ var Controllers = require('./Controllers'), hasManyResource = require('./associations/has-many'), belongsToResource = require('./associations/belongs-to'), belongsToManyResource = require('./associations/belongs-to-many'), - _ = require('lodash'); + _ = require('lodash'), + getKeys = require('./util').keys; var Resource = function(options) { _.defaults(options, { @@ -41,8 +42,8 @@ var Resource = function(options) { this.excludeAttributes = options.excludeAttributes; if (_.isArray(options.excludeAttributes)) { this.attributes = (!options.excludeAttributes.length) ? - Object.keys(this.model.rawAttributes) : - Object.keys(this.model.rawAttributes).filter(function(attr) { + getKeys(this.model.rawAttributes) : + getKeys(this.model.rawAttributes).filter(function(attr) { return options.excludeAttributes.indexOf(attr) === -1; }); } else if (_.isObject(options.excludeAttributes)) { @@ -55,8 +56,8 @@ var Resource = function(options) { }); if (_.has(options.excludeAttributes, "own") && _.isArray(options.excludeAttributes.own)) { this.attributes = (!options.excludeAttributes.own.length) ? - Object.keys(this.model.rawAttributes) : - Object.keys(this.model.rawAttributes).filter(function(attr) { + getKeys(this.model.rawAttributes) : + getKeys(this.model.rawAttributes).filter(function(attr) { return options.excludeAttributes.own.indexOf(attr) === -1; }); // backward compatible and not breaking other stuff! @@ -64,7 +65,7 @@ var Resource = function(options) { } } else { - this.attributes = Object.keys(this.model.rawAttributes); + this.attributes = getKeys(this.model.rawAttributes); } } @@ -211,7 +212,7 @@ function autoAssociate(resource) { && _.isArray(resource.associationOptions[association.as].excludeAttributes)) { assocAttr = (!resource.associationOptions[association.as].excludeAttributes.length) ? association.target.rawAttributes : - Object.keys(association.target.rawAttributes).filter(function(attr) { + getKeys(association.target.rawAttributes).filter(function(attr) { return resource.associationOptions[association.as].excludeAttributes.indexOf(attr) === -1; }); } diff --git a/lib/util.js b/lib/util.js new file mode 100644 index 0000000..4291757 --- /dev/null +++ b/lib/util.js @@ -0,0 +1,10 @@ +'use strict'; + +function keys(obj) { + var symbolKeys = Object.getOwnPropertySymbols ? Object.getOwnPropertySymbols(obj) : []; + return Object.keys(obj).concat(symbolKeys); +} + +module.exports = { + keys: keys +}; diff --git a/tests/util.test.js b/tests/util.test.js new file mode 100644 index 0000000..a606b1a --- /dev/null +++ b/tests/util.test.js @@ -0,0 +1,27 @@ +'use strict'; + +var util = require('../lib/util'); +var expect = require('chai').expect; + +describe('Util', function () { + describe('keys()', function() { + it("should return a simple object's keys", function () { + expect(util.keys({some: 'value'})).to.deep.equal(['some']); + expect(util.keys({some: 'value', an: 'other value'})).to.deep.equal(['some', 'an']); + }); + + it("should return an object's symbol keys", function () { + var obj = {}; + var symbol = Symbol('a_symbol'); + obj[symbol] = 'a value'; + expect(util.keys(obj)).to.deep.equal([symbol]); + }); + + it("should mix string and symbol keys", function () { + var obj = {some: 'value'}; + var symbol = Symbol('a_symbol'); + obj[symbol] = 'a value'; + expect(util.keys(obj)).to.deep.equal(['some', symbol]); + }); + }); +}); From 1f99495848fc49557ef752b8b0415838079999e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gabriel=20Th=C3=A9ron?= Date: Wed, 18 Oct 2017 14:06:34 +0200 Subject: [PATCH 3/5] Uses Reflect.ownKeys() for keys() if available --- lib/util.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/util.js b/lib/util.js index 4291757..e004676 100644 --- a/lib/util.js +++ b/lib/util.js @@ -1,8 +1,13 @@ 'use strict'; function keys(obj) { - var symbolKeys = Object.getOwnPropertySymbols ? Object.getOwnPropertySymbols(obj) : []; - return Object.keys(obj).concat(symbolKeys); + if (typeof Reflect === 'object') { + return Reflect.ownKeys(obj); + } + else { + var getOwnExists = typeof Object.getOwnPropertySymbols === 'function' && typeof Object.getOwnPropertyNames === 'function'; + return getOwnExists ? Object.getOwnPropertyNames(obj).concat(Object.getOwnPropertySymbols(obj)) : Object.keys(obj); + } } module.exports = { From d60990df9c0e8b22bb45845174a74ab5ebc0e2fc Mon Sep 17 00:00:00 2001 From: Panos Michail Date: Fri, 20 Oct 2017 12:31:37 +0300 Subject: [PATCH 4/5] Usage of sequelize Symbol operators --- lib/Controllers/list.js | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/Controllers/list.js b/lib/Controllers/list.js index e181226..2cee086 100644 --- a/lib/Controllers/list.js +++ b/lib/Controllers/list.js @@ -35,7 +35,6 @@ List.prototype._safeishParse = function(value, type, sequelize) { } }; -var stringOperators = /like|iLike|notLike|notILike/; List.prototype.fetch = function(req, res, context) { var self = this, model = this.model, @@ -48,6 +47,10 @@ List.prototype.fetch = function(req, res, context) { count = +context.count || +req.query.count || defaultCount, offset = +context.offset || +req.query.offset || 0; + var stringOperators = [ + Sequelize.Op.like, Sequelize.Op.iLike, Sequelize.Op.notLike, Sequelize.Op.notILike, + ]; + // only look up attributes we care about options.attributes = options.attributes || this.resource.attributes; @@ -72,11 +75,11 @@ List.prototype.fetch = function(req, res, context) { var searchParam = searchData.param; if (_.has(req.query, searchParam)) { var search = []; - var searchOperator = searchData.operator || '$like'; + var searchOperator = searchData.operator || Sequelize.Op.like; var searchAttributes = searchData.attributes || getKeys(model.rawAttributes); searchAttributes.forEach(function(attr) { - if(stringOperators.test(searchOperator)){ + if(stringOperators.indexOf(searchOperator) !== -1){ var attrType = model.rawAttributes[attr].type; if (!(attrType instanceof Sequelize.STRING) && !(attrType instanceof Sequelize.TEXT)) { @@ -90,7 +93,7 @@ List.prototype.fetch = function(req, res, context) { var item = {}; var query = {}; var searchString; - if (!~searchOperator.toLowerCase().indexOf('like')) { + if (searchOperator !== Sequelize.Op.like) { searchString = req.query[searchParam]; } else { searchString = '%' + req.query[searchParam] + '%'; From 35c15bca86a6047256f558744a312afb0495b111 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gabriel=20Th=C3=A9ron?= Date: Fri, 20 Oct 2017 15:08:16 +0200 Subject: [PATCH 5/5] Bumps the sequelize dependency to 4.15.0 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 0311c6b..04eab9d 100644 --- a/package.json +++ b/package.json @@ -47,7 +47,7 @@ "mocha": "^2.2.5", "request": "^2.60.0", "restify": "^4.0.3", - "sequelize": "^3.5.1", + "sequelize": "^4.15.0", "sqlite3": "^3.0.9" } }