From 332579ec873c63b41abb6d7de880d1d65537bda6 Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Sat, 21 Jun 2014 11:44:33 -0700 Subject: [PATCH 1/7] Synchronize with cachedRelations --- lib/relation-definition.js | 139 ++++++++++++++++++++++++++++--------- 1 file changed, 108 insertions(+), 31 deletions(-) diff --git a/lib/relation-definition.js b/lib/relation-definition.js index 13366d7a..fe4a6d7d 100644 --- a/lib/relation-definition.js +++ b/lib/relation-definition.js @@ -60,10 +60,10 @@ function RelationDefinition(definition) { this.type = normalizeType(definition.type); assert(this.type, 'Invalid relation type: ' + definition.type); this.modelFrom = definition.modelFrom; - assert(this.modelFrom); + assert(this.modelFrom, 'Source model is required'); this.keyFrom = definition.keyFrom; this.modelTo = definition.modelTo; - assert(this.modelTo); + assert(this.modelTo, 'Target model is required'); this.keyTo = definition.keyTo; this.modelThrough = definition.modelThrough; this.keyThrough = definition.keyThrough; @@ -106,6 +106,15 @@ function Relation(definition, modelInstance) { this.modelInstance = modelInstance; } +Relation.prototype.resetCache = function (cache) { + cache = cache || undefined; + this.modelInstance.__cachedRelations[this.definition.name] = cache; +}; + +Relation.prototype.getCache = function () { + return this.modelInstance.__cachedRelations[this.definition.name]; +}; + /** * HasMany subclass * @param {RelationDefinition|Object} definition @@ -124,6 +133,39 @@ function HasMany(definition, modelInstance) { util.inherits(HasMany, Relation); +HasMany.prototype.removeFromCache = function (id) { + var cache = this.modelInstance.__cachedRelations[this.definition.name]; + var idName = this.definition.modelTo.definition.idName(); + if (Array.isArray(cache)) { + for (var i = 0, n = cache.length; i < n; i++) { + if (cache[i][idName] === id) { + return cache.splice(i, 1); + } + } + } + return null; +}; + +HasMany.prototype.addToCache = function (inst) { + if (!inst) { + return; + } + var cache = this.modelInstance.__cachedRelations[this.definition.name]; + if (cache === undefined) { + cache = this.modelInstance.__cachedRelations[this.definition.name] = []; + } + var idName = this.definition.modelTo.definition.idName(); + if (Array.isArray(cache)) { + for (var i = 0, n = cache.length; i < n; i++) { + if (cache[i][idName] === inst[idName]) { + cache[i] = inst; + return; + } + } + cache.push(inst); + } +}; + /** * HasManyThrough subclass * @param {RelationDefinition|Object} definition @@ -210,7 +252,7 @@ function findBelongsTo(modelFrom, modelTo, keyTo) { var rel = relations[keys[k]]; if (rel.type === RelationTypes.belongsTo && rel.modelTo === modelTo && - rel.keyTo === keyTo) { + (keyTo === undefined || rel.keyTo === keyTo)) { return rel.keyFrom; } } @@ -356,6 +398,7 @@ HasMany.prototype.findById = function (id, cb) { }; HasMany.prototype.destroyById = function (id, cb) { + var self = this; var modelTo = this.definition.modelTo; var fk = this.definition.keyTo; var pk = this.definition.keyFrom; @@ -369,6 +412,7 @@ HasMany.prototype.destroyById = function (id, cb) { } // Check if the foreign key matches the primary key if (inst[fk] && inst[fk].toString() === modelInstance[pk].toString()) { + self.removeFromCache(inst[fk]); inst.destroy(cb); } else { cb(new Error('Permission denied: foreign key does not match the primary key')); @@ -379,6 +423,7 @@ HasMany.prototype.destroyById = function (id, cb) { // Create an instance of the target model and connect it to the instance of // the source model by creating an instance of the through model HasManyThrough.prototype.create = function create(data, done) { + var self = this; var definition = this.definition; var modelTo = definition.modelTo; var modelThrough = definition.modelThrough; @@ -391,27 +436,28 @@ HasManyThrough.prototype.create = function create(data, done) { var modelInstance = this.modelInstance; // First create the target model - modelTo.create(data, function (err, ac) { + modelTo.create(data, function (err, to) { if (err) { - return done && done(err, ac); + return done && done(err, to); } // The primary key for the target model - var pk2 = modelTo.dataSource.idName(modelTo.modelName) || 'id'; + var pk2 = definition.modelTo.definition.idName(); var fk1 = findBelongsTo(modelThrough, definition.modelFrom, definition.keyFrom); var fk2 = findBelongsTo(modelThrough, definition.modelTo, pk2); var d = {}; d[fk1] = modelInstance[definition.keyFrom]; - d[fk2] = ac[pk2]; + d[fk2] = to[pk2]; // Then create the through model - modelThrough.create(d, function (e) { + modelThrough.create(d, function (e, through) { if (e) { // Undo creation of the target model - ac.destroy(function () { + to.destroy(function () { done && done(e); }); } else { - done && done(err, ac); + self.addToCache(to); + done && done(err, to); } }); }); @@ -422,9 +468,9 @@ HasManyThrough.prototype.create = function create(data, done) { * @param {Object|ID} acInst The actual instance or id value */ HasManyThrough.prototype.add = function (acInst, done) { + var self = this; var definition = this.definition; var modelThrough = definition.modelThrough; - var modelTo = definition.modelTo; var pk1 = definition.keyFrom; var data = {}; @@ -434,7 +480,7 @@ HasManyThrough.prototype.add = function (acInst, done) { definition.keyFrom); // The primary key for the target model - var pk2 = modelTo.dataSource.idName(modelTo.modelName) || 'id'; + var pk2 = definition.modelTo.definition.idName(); var fk2 = findBelongsTo(modelThrough, definition.modelTo, pk2); @@ -445,7 +491,14 @@ HasManyThrough.prototype.add = function (acInst, done) { data[fk2] = acInst[pk2] || acInst; // Create an instance of the through model - modelThrough.findOrCreate({where: query}, data, done); + modelThrough.findOrCreate({where: query}, data, function(err, ac) { + if(!err) { + if (acInst instanceof definition.modelTo) { + self.addToCache(acInst); + } + } + done(err, ac); + }); }; /** @@ -453,13 +506,30 @@ HasManyThrough.prototype.add = function (acInst, done) { * @param {Object|ID) acInst The actual instance or id value */ HasManyThrough.prototype.remove = function (acInst, done) { - var modelThrough = this.definition.modelThrough; - var fk2 = this.definition.keyThrough; - var pk = this.definition.keyFrom; + var self = this; + var definition = this.definition; + var modelThrough = definition.modelThrough; + var pk1 = definition.keyFrom; - var q = {}; - q[fk2] = acInst[pk] || acInst; - modelThrough.deleteAll(q, done ); + var query = {}; + + var fk1 = findBelongsTo(modelThrough, definition.modelFrom, + definition.keyFrom); + + // The primary key for the target model + var pk2 = definition.modelTo.definition.idName(); + + var fk2 = findBelongsTo(modelThrough, definition.modelTo, pk2); + + query[fk1] = this.modelInstance[pk1]; + query[fk2] = acInst[pk2] || acInst; + + modelThrough.deleteAll(query, function (err) { + if (!err) { + self.removeFromCache(query[fk2]); + } + done(err); + }); }; @@ -547,6 +617,7 @@ RelationDefinition.belongsTo = function (modelFrom, modelTo, params) { }; BelongsTo.prototype.create = function(targetModelData, cb) { + var self = this; var modelTo = this.definition.modelTo; var fk = this.definition.keyTo; var pk = this.definition.keyFrom; @@ -555,6 +626,7 @@ BelongsTo.prototype.create = function(targetModelData, cb) { modelTo.create(targetModelData, function(err, targetModel) { if(!err) { modelInstance[fk] = targetModel[pk]; + self.resetCache(targetModel); cb && cb(err, targetModel); } else { cb && cb(err); @@ -579,11 +651,11 @@ BelongsTo.prototype.build = function(targetModelData) { * @returns {*} */ BelongsTo.prototype.related = function (refresh, params) { + var self = this; var modelTo = this.definition.modelTo; var pk = this.definition.keyTo; var fk = this.definition.keyFrom; var modelInstance = this.modelInstance; - var relationName = this.definition.name; if (arguments.length === 1) { params = refresh; @@ -593,13 +665,12 @@ BelongsTo.prototype.related = function (refresh, params) { } var cachedValue; - if (!refresh && modelInstance.__cachedRelations - && (modelInstance.__cachedRelations[relationName] !== undefined)) { - cachedValue = modelInstance.__cachedRelations[relationName]; + if (!refresh) { + cachedValue = self.getCache(); } if (params instanceof ModelBaseClass) { // acts as setter modelInstance[fk] = params[pk]; - modelInstance.__cachedRelations[relationName] = params; + self.resetCache(params); } else if (typeof params === 'function') { // acts as async getter var cb = params; if (cachedValue === undefined) { @@ -612,9 +683,10 @@ BelongsTo.prototype.related = function (refresh, params) { } // Check if the foreign key matches the primary key if (inst[pk] === modelInstance[fk]) { + self.resetCache(inst); cb(null, inst); } else { - cb(new Error('Permission denied')); + cb(new Error('Permission denied: foreign key does not match the primary key')); } }); return modelInstance[fk]; @@ -626,7 +698,7 @@ BelongsTo.prototype.related = function (refresh, params) { return modelInstance[fk]; } else { // setter modelInstance[fk] = params; - delete modelInstance.__cachedRelations[relationName]; + self.resetCache(); } }; @@ -742,15 +814,19 @@ RelationDefinition.hasOne = function (modelFrom, modelTo, params) { * @param {Object} The newly created target model instance */ HasOne.prototype.create = function(targetModelData, cb) { + var self = this; var modelTo = this.definition.modelTo; var fk = this.definition.keyTo; var pk = this.definition.keyFrom; var modelInstance = this.modelInstance; + var relationName = this.definition.name targetModelData = targetModelData || {}; targetModelData[fk] = modelInstance[pk]; modelTo.create(targetModelData, function(err, targetModel) { if(!err) { + // Refresh the cache + self.resetCache(targetModel); cb && cb(err, targetModel); } else { cb && cb(err); @@ -784,6 +860,7 @@ HasOne.prototype.build = function(targetModelData) { * @returns {Object} */ HasOne.prototype.related = function (refresh, params) { + var self = this; var modelTo = this.definition.modelTo; var fk = this.definition.keyTo; var pk = this.definition.keyFrom; @@ -798,13 +875,12 @@ HasOne.prototype.related = function (refresh, params) { } var cachedValue; - if (!refresh && modelInstance.__cachedRelations - && (modelInstance.__cachedRelations[relationName] !== undefined)) { - cachedValue = modelInstance.__cachedRelations[relationName]; + if (!refresh) { + cachedValue = self.getCache(); } if (params instanceof ModelBaseClass) { // acts as setter params[fk] = modelInstance[pk]; - modelInstance.__cachedRelations[relationName] = params; + self.resetCache(params); } else if (typeof params === 'function') { // acts as async getter var cb = params; if (cachedValue === undefined) { @@ -819,6 +895,7 @@ HasOne.prototype.related = function (refresh, params) { } // Check if the foreign key matches the primary key if (inst[fk] === modelInstance[pk]) { + self.resetCache(inst); cb(null, inst); } else { cb(new Error('Permission denied')); @@ -833,6 +910,6 @@ HasOne.prototype.related = function (refresh, params) { return modelInstance[pk]; } else { // setter params[fk] = modelInstance[pk]; - delete modelInstance.__cachedRelations[relationName]; + self.resetCache(); } }; From 3edee5c4c59a4ccc3e72d39d84c929ad29185eda Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Sat, 21 Jun 2014 12:53:06 -0700 Subject: [PATCH 2/7] Work around for Date default See https://github.com/strongloop/loopback-connector-postgresql/issues/15 --- lib/model.js | 14 +++++++++++--- test/loopback-dl.test.js | 4 +++- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/lib/model.js b/lib/model.js index aaf5e29f..51a59397 100644 --- a/lib/model.js +++ b/lib/model.js @@ -158,10 +158,18 @@ ModelBaseClass.prototype._initProperties = function (data, options) { var def = properties[p]['default']; if (def !== undefined) { if (typeof def === 'function') { - self.__data[p] = def(); - } else { - self.__data[p] = def; + if (def === Date) { + // FIXME: We should coerce the value in general + // This is a work around to {default: Date} + // Date() will return a string instead of Date + def = new Date(); + } else { + def = def(); + } } + // FIXME: We should coerce the value + // will implement it after we refactor the PropertyDefinition + self.__data[p] = def; } } diff --git a/test/loopback-dl.test.js b/test/loopback-dl.test.js index d26647ce..3fa1ccd8 100644 --- a/test/loopback-dl.test.js +++ b/test/loopback-dl.test.js @@ -264,7 +264,7 @@ describe('DataSource define model', function () { name: String, bio: ModelBuilder.Text, approved: Boolean, - joinedAt: Date, + joinedAt: {type: Date, default: Date}, age: Number }); @@ -280,6 +280,8 @@ describe('DataSource define model', function () { assert.equal(user.name, 'Joe'); assert.equal(user.group, 'G1'); + assert(user.joinedAt instanceof Date); + // setup relationships User.hasMany(Post, {as: 'posts', foreignKey: 'userId'}); From a1d3e72046763ce9936da809682b440c3e9edd29 Mon Sep 17 00:00:00 2001 From: Rand McKinney Date: Tue, 24 Jun 2014 16:15:35 -0700 Subject: [PATCH 3/7] Update link to doc --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 1a14aa30..66f41ee5 100644 --- a/README.md +++ b/README.md @@ -5,8 +5,8 @@ for interacting with databases, REST APIs, and other data sources. It was initially forked from [JugglingDB](https://github.com/1602/jugglingdb). **For full documentation, see the official StrongLoop documentation**: - * [Data sources and connectors](http://docs.strongloop.com/display/DOC/Data+sources+and+connectors) - * [Data Source Juggler](http://docs.strongloop.com/display/DOC/Data+Source+Juggler). + * [Data sources and connectors](http://docs.strongloop.com/display/LB/Data+sources+and+connectors) + * [Creating data sources and connected models](http://docs.strongloop.com/display/LB/Creating+data+sources+and+connected+models). ## Installation From a3ed1a575e7dff0d8832b4634cf372b50f75957f Mon Sep 17 00:00:00 2001 From: Ritchie Martori Date: Thu, 26 Jun 2014 14:48:27 -0700 Subject: [PATCH 4/7] Add "hasOne" to relationTypes --- lib/datasource.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/datasource.js b/lib/datasource.js index 7c8b4190..e1841968 100644 --- a/lib/datasource.js +++ b/lib/datasource.js @@ -364,7 +364,7 @@ function isModelClass(cls) { return cls.prototype instanceof ModelBaseClass; } -DataSource.relationTypes = ['belongsTo', 'hasMany', 'hasAndBelongsToMany']; +DataSource.relationTypes = ['belongsTo', 'hasMany', 'hasAndBelongsToMany', 'hasOne']; function isModelDataSourceAttached(model) { return model && (!model.settings.unresolved) && (model.dataSource instanceof DataSource); From 45154913188c09a00f06e49502227343726fc4a7 Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Thu, 26 Jun 2014 23:38:04 -0700 Subject: [PATCH 5/7] Make sure type of the foreign key match the primary key See https://github.com/strongloop/loopback/issues/353 --- lib/datasource.js | 28 +++++++++------- lib/relation-definition.js | 69 +++++++++++++++++++++++++++++++++----- lib/scope.js | 4 ++- test/relations.test.js | 4 +-- 4 files changed, 81 insertions(+), 24 deletions(-) diff --git a/lib/datasource.js b/lib/datasource.js index 7c8b4190..64d0c210 100644 --- a/lib/datasource.js +++ b/lib/datasource.js @@ -1579,22 +1579,26 @@ DataSource.prototype.idProperty = function (modelName) { * @param {String} foreignClassName The foreign model name */ DataSource.prototype.defineForeignKey = function defineForeignKey(className, key, foreignClassName) { - // quit if key already defined - if (this.getModelDefinition(className).rawProperties[key]) return; - - var defaultType = Number; - if (foreignClassName) { - var foreignModel = this.getModelDefinition(foreignClassName); - var pkName = foreignModel && foreignModel.idName(); - if (pkName) { - defaultType = foreignModel.properties[pkName].type; - } + var pkType = null; + var foreignModel = this.getModelDefinition(foreignClassName); + var pkName = foreignModel && foreignModel.idName(); + if (pkName) { + pkType = foreignModel.properties[pkName].type; } + var model = this.getModelDefinition(className); + if (model.properties[key]) { + if (pkType) { + // Reset the type of the foreign key + model.rawProperties[key].type = model.properties[key].type = pkType; + } + return; + } + if (this.connector.defineForeignKey) { var cb = function (err, keyType) { if (err) throw err; // Add the foreign key property to the data source _models - this.defineProperty(className, key, {type: keyType || defaultType}); + this.defineProperty(className, key, {type: keyType || pkType}); }.bind(this); switch (this.connector.defineForeignKey.length) { case 4: @@ -1607,7 +1611,7 @@ DataSource.prototype.defineForeignKey = function defineForeignKey(className, key } } else { // Add the foreign key property to the data source _models - this.defineProperty(className, key, {type: defaultType}); + this.defineProperty(className, key, {type: pkType}); } }; diff --git a/lib/relation-definition.js b/lib/relation-definition.js index fe4a6d7d..c50cf0d6 100644 --- a/lib/relation-definition.js +++ b/lib/relation-definition.js @@ -336,7 +336,7 @@ RelationDefinition.hasMany = function hasMany(modelFrom, modelTo, params) { if (!params.through) { // obviously, modelTo should have attribute called `fk` - modelTo.dataSource.defineForeignKey(modelTo.modelName, fk, this.modelName); + modelTo.dataSource.defineForeignKey(modelTo.modelName, fk, modelFrom.modelName); } var scopeMethods = { @@ -348,6 +348,9 @@ RelationDefinition.hasMany = function hasMany(modelFrom, modelTo, params) { scopeMethods.create = scopeMethod(definition, 'create'); scopeMethods.add = scopeMethod(definition, 'add'); scopeMethods.remove = scopeMethod(definition, 'remove'); + } else { + scopeMethods.create = scopeMethod(definition, 'create'); + scopeMethods.build = scopeMethod(definition, 'build'); } // Mix the property and scoped methods into the prototype class @@ -623,6 +626,11 @@ BelongsTo.prototype.create = function(targetModelData, cb) { var pk = this.definition.keyFrom; var modelInstance = this.modelInstance; + if (typeof targetModelData === 'function' && !cb) { + cb = targetModelData; + targetModelData = {}; + } + modelTo.create(targetModelData, function(err, targetModel) { if(!err) { modelInstance[fk] = targetModel[pk]; @@ -695,7 +703,7 @@ BelongsTo.prototype.related = function (refresh, params) { return cachedValue; } } else if (params === undefined) { // acts as sync getter - return modelInstance[fk]; + return cachedValue; } else { // setter modelInstance[fk] = params; self.resetCache(); @@ -813,33 +821,77 @@ RelationDefinition.hasOne = function (modelFrom, modelTo, params) { * @param {String|Object} err Error string or object * @param {Object} The newly created target model instance */ -HasOne.prototype.create = function(targetModelData, cb) { +HasOne.prototype.create = function (targetModelData, cb) { var self = this; var modelTo = this.definition.modelTo; var fk = this.definition.keyTo; var pk = this.definition.keyFrom; var modelInstance = this.modelInstance; - var relationName = this.definition.name + if (typeof targetModelData === 'function' && !cb) { + cb = targetModelData; + targetModelData = {}; + } + targetModelData = targetModelData || {}; + targetModelData[fk] = modelInstance[pk]; + var query = {where: {}}; + query.where[fk] = targetModelData[fk] + modelTo.findOne(query, function(err, result) { + if(err) { + cb(err); + } else if(result) { + cb(new Error('HasOne relation cannot create more than one instance of ' + + modelTo.modelName)); + } else { + modelTo.create(targetModelData, function (err, targetModel) { + if (!err) { + // Refresh the cache + self.resetCache(targetModel); + cb && cb(err, targetModel); + } else { + cb && cb(err); + } + }); + } + }); +}; + +/** + * Create a target model instance + * @param {Object} targetModelData The target model data + * @callback {Function} [cb] Callback function + * @param {String|Object} err Error string or object + * @param {Object} The newly created target model instance + */ +HasMany.prototype.create = function (targetModelData, cb) { + var self = this; + var modelTo = this.definition.modelTo; + var fk = this.definition.keyTo; + var pk = this.definition.keyFrom; + var modelInstance = this.modelInstance; + + if (typeof targetModelData === 'function' && !cb) { + cb = targetModelData; + targetModelData = {}; + } targetModelData = targetModelData || {}; targetModelData[fk] = modelInstance[pk]; modelTo.create(targetModelData, function(err, targetModel) { if(!err) { // Refresh the cache - self.resetCache(targetModel); + self.addToCache(targetModel); cb && cb(err, targetModel); } else { cb && cb(err); } }); }; - /** * Build a target model instance * @param {Object} targetModelData The target model data * @returns {Object} The newly built target model instance */ -HasOne.prototype.build = function(targetModelData) { +HasMany.prototype.build = HasOne.prototype.build = function(targetModelData) { var modelTo = this.definition.modelTo; var pk = this.definition.keyFrom; var fk = this.definition.keyTo; @@ -865,7 +917,6 @@ HasOne.prototype.related = function (refresh, params) { var fk = this.definition.keyTo; var pk = this.definition.keyFrom; var modelInstance = this.modelInstance; - var relationName = this.definition.name; if (arguments.length === 1) { params = refresh; @@ -907,7 +958,7 @@ HasOne.prototype.related = function (refresh, params) { return cachedValue; } } else if (params === undefined) { // acts as sync getter - return modelInstance[pk]; + return cachedValue; } else { // setter params[fk] = modelInstance[pk]; self.resetCache(); diff --git a/lib/scope.js b/lib/scope.js index 8a4485df..5267ce8d 100644 --- a/lib/scope.js +++ b/lib/scope.js @@ -202,7 +202,9 @@ function defineScope(cls, targetClass, name, params, methods) { var prop = targetClass.definition.properties[i]; if (prop) { var val = where[i]; - if (typeof val !== 'object' || val instanceof prop.type) { + if (typeof val !== 'object' || val instanceof prop.type + || prop.type.name === 'ObjectID') // MongoDB key + { // Only pick the {propertyName: propertyValue} data[i] = where[i]; } diff --git a/test/relations.test.js b/test/relations.test.js index 4e1f5873..b8f34254 100644 --- a/test/relations.test.js +++ b/test/relations.test.js @@ -155,7 +155,7 @@ describe('relations', function () { should.not.exist(e); should.exist(l); l.should.be.an.instanceOf(List); - todo.list().should.equal(l.id); + todo.list().id.should.equal(l.id); done(); }); }); @@ -206,7 +206,7 @@ describe('relations', function () { should.not.exist(e); should.exist(act); act.should.be.an.instanceOf(Account); - supplier.account().should.equal(act.id); + supplier.account().id.should.equal(act.id); done(); }); }); From e0c76199085eb09b94411e13171a20d033ccde48 Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Thu, 26 Jun 2014 23:40:20 -0700 Subject: [PATCH 6/7] Normalize filter.order and enforce more checks --- lib/connectors/memory.js | 6 +++ lib/dao.js | 112 ++++++++++++++++++++++++++++++++------- test/memory.test.js | 58 ++++++++++++++++++++ 3 files changed, 156 insertions(+), 20 deletions(-) diff --git a/lib/connectors/memory.js b/lib/connectors/memory.js index b568a0f3..7441ddd6 100644 --- a/lib/connectors/memory.js +++ b/lib/connectors/memory.js @@ -270,6 +270,12 @@ Memory.prototype.all = function all(model, filter, callback) { }.bind(this)); if (filter) { + if (!filter.order) { + var idNames = this.idNames(model); + if (idNames && idNames.length) { + filter.order = idNames; + } + } // do we need some sorting? if (filter.order) { var orders = filter.order; diff --git a/lib/dao.js b/lib/dao.js index 745b6996..131be88c 100644 --- a/lib/dao.js +++ b/lib/dao.js @@ -434,6 +434,48 @@ DataAccessObject._normalize = function (filter) { filter.skip = offset; } + if (filter.order) { + var order = filter.order; + if (!Array.isArray(order)) { + order = [order]; + } + var fields = []; + for (var i = 0, m = order.length; i < m; i++) { + if (typeof order[i] === 'string') { + // Normalize 'f1 ASC, f2 DESC, f3' to ['f1 ASC', 'f2 DESC', 'f3'] + var tokens = order[i].split(/(?:\s*,\s*)+/); + for (var t = 0, n = tokens.length; t < n; t++) { + var token = tokens[t]; + if (token.length === 0) { + // Skip empty token + continue; + } + var parts = token.split(/\s+/); + if (parts.length >= 2) { + var dir = parts[1].toUpperCase(); + if (dir === 'ASC' || dir === 'DESC') { + token = parts[0] + ' ' + dir; + } else { + err = new Error(util.format('The order %j has invalid direction', token)); + err.statusCode = 400; + throw err; + } + } + fields.push(token); + } + } else { + err = new Error(util.format('The order %j is not valid', order[i])); + err.statusCode = 400; + throw err; + } + } + if (fields.length === 1 && typeof filter.order === 'string') { + filter.order = fields[0]; + } else { + filter.order = fields; + } + } + // normalize fields as array of included property names if (filter.fields) { filter.fields = fieldsToArray(filter.fields, @@ -445,6 +487,25 @@ DataAccessObject._normalize = function (filter) { return filter; }; +function DateType(arg) { + return new Date(arg); +} + +function BooleanType(val) { + if (val === 'true') { + return true; + } else if (val === 'false') { + return false; + } else { + return Boolean(val); + } +} + +function NumberType(val) { + var num = Number(val); + return !isNaN(num) ? num : val; +} + /* * Coerce values based the property types * @param {Object} where The where clause @@ -470,11 +531,11 @@ DataAccessObject._coerce = function (where) { if (p === 'and' || p === 'or' || p === 'nor') { var clauses = where[p]; if (Array.isArray(clauses)) { - for (var i = 0; i < clauses.length; i++) { - self._coerce(clauses[i]); + for (var k = 0; k < clauses.length; k++) { + self._coerce(clauses[k]); } } else { - err = new Error(util.format('The %p operator has invalid clauses %j', p, clauses)); + err = new Error(util.format('The %s operator has invalid clauses %j', p, clauses)); err.statusCode = 400; throw err; } @@ -488,30 +549,16 @@ DataAccessObject._coerce = function (where) { DataType = DataType[0]; } if (DataType === Date) { - var OrigDate = Date; - DataType = function Date(arg) { - return new OrigDate(arg); - }; + DataType = DateType; } else if (DataType === Boolean) { - DataType = function (val) { - if (val === 'true') { - return true; - } else if (val === 'false') { - return false; - } else { - return Boolean(val); - } - }; + DataType = BooleanType; } else if (DataType === Number) { // This fixes a regression in mongodb connector // For numbers, only convert it produces a valid number // LoopBack by default injects a number id. We should fix it based // on the connector's input, for example, MongoDB should use string // while RDBs typically use number - DataType = function (val) { - var num = Number(val); - return !isNaN(num) ? num : val; - }; + DataType = NumberType; } if (!DataType) { @@ -543,6 +590,31 @@ DataAccessObject._coerce = function (where) { if (op in val) { val = val[op]; operator = op; + switch(operator) { + case 'inq': + case 'nin': + if (!Array.isArray(val)) { + err = new Error(util.format('The %s property has invalid clause %j', p, where[p])); + err.statusCode = 400; + throw err; + } + break; + case 'between': + if (!Array.isArray(val) || val.length !== 2) { + err = new Error(util.format('The %s property has invalid clause %j', p, where[p])); + err.statusCode = 400; + throw err; + } + break; + case 'like': + case 'nlike': + if (!(typeof val === 'string' || val instanceof RegExp)) { + err = new Error(util.format('The %s property has invalid clause %j', p, where[p])); + err.statusCode = 400; + throw err; + } + break; + } break; } } diff --git a/test/memory.test.js b/test/memory.test.js index 4016209e..40283b82 100644 --- a/test/memory.test.js +++ b/test/memory.test.js @@ -141,6 +141,64 @@ describe('Memory connector', function () { }); }); + it('should throw if the like value is not string or regexp', function (done) { + User.find({where: {name: {like: 123}}}, function (err, posts) { + should.exist(err); + done(); + }); + }); + + it('should throw if the nlike value is not string or regexp', function (done) { + User.find({where: {name: {nlike: 123}}}, function (err, posts) { + should.exist(err); + done(); + }); + }); + + it('should throw if the inq value is not an array', function (done) { + User.find({where: {name: {inq: '12'}}}, function (err, posts) { + should.exist(err); + done(); + }); + }); + + it('should throw if the nin value is not an array', function (done) { + User.find({where: {name: {nin: '12'}}}, function (err, posts) { + should.exist(err); + done(); + }); + }); + + it('should throw if the between value is not an array', function (done) { + User.find({where: {name: {between: '12'}}}, function (err, posts) { + should.exist(err); + done(); + }); + }); + + it('should throw if the between value is not an array of length 2', function (done) { + User.find({where: {name: {between: ['12']}}}, function (err, posts) { + should.exist(err); + done(); + }); + }); + + it('support order with multiple fields', function (done) { + User.find({order: 'vip ASC, seq DESC'}, function (err, posts) { + should.not.exist(err); + posts[0].seq.should.be.eql(4); + posts[1].seq.should.be.eql(3); + done(); + }); + }); + + it('should throw if order has wrong direction', function (done) { + User.find({order: 'seq ABC'}, function (err, posts) { + should.exist(err); + done(); + }); + }); + function seed(done) { var beatles = [ { From e7a97da3bfea18449413bb6460c6dfbfd52aae87 Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Fri, 27 Jun 2014 10:12:10 -0700 Subject: [PATCH 7/7] Bump version and update deps --- package.json | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/package.json b/package.json index 8e633c6d..ace15d4e 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "loopback-datasource-juggler", - "version": "1.6.1", + "version": "1.6.2", "description": "LoopBack DataSoure Juggler", "keywords": [ "StrongLoop", @@ -24,14 +24,14 @@ ], "devDependencies": { "should": "~1.2.2", - "mocha": "~1.18.2" + "mocha": "~1.20.1" }, "dependencies": { "async": "~0.9.0", - "inflection": "~1.3.5", - "traverse": "~0.6.6", + "debug": "~1.0.2", + "inflection": "~1.3.7", "qs": "~0.6.6", - "debug": "~0.8.1" + "traverse": "~0.6.6" }, "license": { "name": "Dual MIT/StrongLoop",