From d1466bffcb13edce5faa2072850bb1d10a443615 Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Sat, 9 Aug 2014 10:58:03 +0200 Subject: [PATCH 01/34] Cleanup mixin tests --- test/mixins.test.js | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/test/mixins.test.js b/test/mixins.test.js index f1dacf9d..e60f7e02 100644 --- a/test/mixins.test.js +++ b/test/mixins.test.js @@ -42,12 +42,15 @@ mixins.define('TimeStamp', timestamps); describe('Model class', function () { - it('should define a mixin', function() { + it('should define mixins', function() { mixins.define('Example', function(Model, options) { Model.prototype.example = function() { return options; }; }); + mixins.define('Demo', function(Model, options) { + Model.demoMixin = options.ok; + }); }); it('should apply a mixin class', function() { @@ -58,7 +61,7 @@ describe('Model class', function () { var memory = new DataSource('mem', {connector: Memory}, modelBuilder); var Item = memory.createModel('Item', { name: 'string' }, { - mixins: { TimeStamp: true, demo: true, Address: true } + mixins: { Address: true } }); var properties = Item.definition.properties; @@ -70,11 +73,12 @@ describe('Model class', function () { it('should apply mixins', function(done) { var memory = new DataSource('mem', {connector: Memory}, modelBuilder); var Item = memory.createModel('Item', { name: 'string' }, { - mixins: { TimeStamp: true, demo: { ok: true } } + mixins: { TimeStamp: true, Demo: { ok: true } } }); Item.mixin('Example', { foo: 'bar' }); - Item.mixin('other'); + + Item.demoMixin.should.be.true; var properties = Item.definition.properties; properties.createdAt.should.eql({ type: Date }); From 1724f350dee9f8f99f95c66d717cbcbb6b8e892b Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Sun, 10 Aug 2014 17:08:31 +0200 Subject: [PATCH 02/34] Refactor modelTo logic into lookupModelTo --- lib/relation-definition.js | 85 +++++++++++++------------------------- 1 file changed, 28 insertions(+), 57 deletions(-) diff --git a/lib/relation-definition.js b/lib/relation-definition.js index bace4705..aa42e086 100644 --- a/lib/relation-definition.js +++ b/lib/relation-definition.js @@ -398,6 +398,25 @@ function lookupModel(models, modelName) { } } +function lookupModelTo(modelFrom, modelTo, params, singularize) { + if ('string' === typeof modelTo) { + params.as = params.as || modelTo; + if (typeof params.model === 'function') { + modelTo = params.model; + } else if (typeof params.model === 'string') { + modelTo = params.model; + } + if (typeof modelTo === 'string') { + var modelToName = (singularize ? i8n.singularize(modelTo) : modelTo).toLowerCase(); + modelTo = lookupModel(modelFrom.dataSource.modelBuilder.models, modelToName) || modelTo; + } + if (typeof modelTo === 'string') { + throw new Error('Could not find "' + modelTo + '" relation for ' + modelFrom.modelName); + } + } + return modelTo; +} + /*! * Normalize polymorphic parameters * @param {Object|String} params Name of the polymorphic relation or params @@ -436,15 +455,7 @@ function polymorphicParams(params) { RelationDefinition.hasMany = function hasMany(modelFrom, modelTo, params) { var thisClassName = modelFrom.modelName; params = params || {}; - if (typeof modelTo === 'string') { - params.as = modelTo; - if (params.model) { - modelTo = params.model; - } else { - var modelToName = i8n.singularize(modelTo).toLowerCase(); - modelTo = lookupModel(modelFrom.dataSource.modelBuilder.models, modelToName); - } - } + modelTo = lookupModelTo(modelFrom, modelTo, params, true); var relationName = params.as || i8n.camelize(modelTo.pluralModelName, true); var fk = params.foreignKey || i8n.camelize(thisClassName + '_id', true); @@ -953,13 +964,7 @@ RelationDefinition.belongsTo = function (modelFrom, modelTo, params) { var discriminator, polymorphic; params = params || {}; if ('string' === typeof modelTo && !params.polymorphic) { - params.as = modelTo; - if (params.model) { - modelTo = params.model; - } else { - var modelToName = modelTo.toLowerCase(); - modelTo = lookupModel(modelFrom.dataSource.modelBuilder.models, modelToName); - } + modelTo = lookupModelTo(modelFrom, modelTo, params); } var idName, relationName, fk; @@ -1182,19 +1187,9 @@ BelongsTo.prototype.related = function (refresh, params) { */ RelationDefinition.hasAndBelongsToMany = function hasAndBelongsToMany(modelFrom, modelTo, params) { params = params || {}; + modelTo = lookupModelTo(modelFrom, modelTo, params, true); + var models = modelFrom.dataSource.modelBuilder.models; - - if ('string' === typeof modelTo) { - params.as = modelTo; - if (params.model) { - modelTo = params.model; - } else { - modelTo = lookupModel(models, i8n.singularize(modelTo)) || modelTo; - } - if (typeof modelTo === 'string') { - throw new Error('Could not find "' + modelTo + '" relation for ' + modelFrom.modelName); - } - } if (!params.through) { if (params.polymorphic) throw new Error('Polymorphic relations need a through model'); @@ -1243,15 +1238,7 @@ RelationDefinition.hasAndBelongsToMany = function hasAndBelongsToMany(modelFrom, */ RelationDefinition.hasOne = function (modelFrom, modelTo, params) { params = params || {}; - if ('string' === typeof modelTo) { - params.as = modelTo; - if (params.model) { - modelTo = params.model; - } else { - var modelToName = modelTo.toLowerCase(); - modelTo = lookupModel(modelFrom.dataSource.modelBuilder.models, modelToName); - } - } + modelTo = lookupModelTo(modelFrom, modelTo, params); var pk = modelFrom.dataSource.idName(modelTo.modelName) || 'id'; var relationName = params.as || i8n.camelize(modelTo.modelName, true); @@ -1476,18 +1463,10 @@ HasOne.prototype.related = function (refresh, params) { }; RelationDefinition.embedsMany = function embedsMany(modelFrom, modelTo, params) { - var thisClassName = modelFrom.modelName; params = params || {}; - if (typeof modelTo === 'string') { - params.as = modelTo; - if (params.model) { - modelTo = params.model; - } else { - var modelToName = i8n.singularize(modelTo).toLowerCase(); - modelTo = lookupModel(modelFrom.dataSource.modelBuilder.models, modelToName); - } - } + modelTo = lookupModelTo(modelFrom, modelTo, params, true); + var thisClassName = modelFrom.modelName; var accessorName = params.as || (i8n.camelize(modelTo.modelName, true) + 'List'); var relationName = params.property || i8n.camelize(modelTo.pluralModelName, true); var fk = modelTo.dataSource.idName(modelTo.modelName) || 'id'; @@ -1925,18 +1904,10 @@ EmbedsMany.prototype.remove = function (acInst, cb) { }; RelationDefinition.referencesMany = function referencesMany(modelFrom, modelTo, params) { - var thisClassName = modelFrom.modelName; params = params || {}; - if (typeof modelTo === 'string') { - params.as = modelTo; - if (params.model) { - modelTo = params.model; - } else { - var modelToName = i8n.singularize(modelTo).toLowerCase(); - modelTo = lookupModel(modelFrom.dataSource.modelBuilder.models, modelToName); - } - } + modelTo = lookupModelTo(modelFrom, modelTo, params, true); + var thisClassName = modelFrom.modelName; var relationName = params.as || i8n.camelize(modelTo.pluralModelName, true); var fk = params.foreignKey || i8n.camelize(modelTo.modelName + '_ids', true); var idName = modelTo.dataSource.idName(modelTo.modelName) || 'id'; From 93fab448bc9fa9f321d4b7c8dbe4cf2b6a7e0222 Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Mon, 11 Aug 2014 13:03:51 +0200 Subject: [PATCH 03/34] Tiny fix: use setAttributes --- lib/relation-definition.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/relation-definition.js b/lib/relation-definition.js index 1171b998..15d78da6 100644 --- a/lib/relation-definition.js +++ b/lib/relation-definition.js @@ -1699,9 +1699,7 @@ EmbedsMany.prototype.updateById = function (fkId, data, cb) { if (inst instanceof modelTo) { if (typeof data === 'object') { - for (var key in data) { - inst[key] = data[key]; - } + inst.setAttributes(data); } var err = inst.isValid() ? null : new ValidationError(inst); if (err && typeof cb === 'function') { From f2c349b5b3c545cb138b5e967bed399c19bf3ea9 Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Mon, 11 Aug 2014 14:43:51 +0200 Subject: [PATCH 04/34] Enable DL definition of embedsMany + referencesMany --- lib/datasource.js | 3 ++- test/loopback-dl.test.js | 20 ++++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/lib/datasource.js b/lib/datasource.js index b488ce72..9602ec4f 100644 --- a/lib/datasource.js +++ b/lib/datasource.js @@ -3,6 +3,7 @@ */ var ModelBuilder = require('./model-builder.js').ModelBuilder; var ModelDefinition = require('./model-definition.js'); +var RelationDefinition = require('./relation-definition.js'); var jutil = require('./jutil'); var utils = require('./utils'); var ModelBaseClass = require('./model.js'); @@ -364,7 +365,7 @@ function isModelClass(cls) { return cls.prototype instanceof ModelBaseClass; } -DataSource.relationTypes = ['belongsTo', 'hasMany', 'hasAndBelongsToMany', 'hasOne']; +DataSource.relationTypes = Object.keys(RelationDefinition.RelationTypes); function isModelDataSourceAttached(model) { return model && (!model.settings.unresolved) && (model.dataSource instanceof DataSource); diff --git a/test/loopback-dl.test.js b/test/loopback-dl.test.js index 18f0e0ad..f388e50e 100644 --- a/test/loopback-dl.test.js +++ b/test/loopback-dl.test.js @@ -903,6 +903,26 @@ describe('Load models with relations', function () { assert(Post.relations['user']); done(); }); + + it('should set up referencesMany relations', function (done) { + var ds = new DataSource('memory'); + + var Post = ds.define('Post', {userId: Number, content: String}); + var User = ds.define('User', {name: String}, {relations: {posts: {type: 'referencesMany', model: 'Post'}}}); + + assert(User.relations['posts']); + done(); + }); + + it('should set up embedsMany relations', function (done) { + var ds = new DataSource('memory'); + + var Post = ds.define('Post', {userId: Number, content: String}); + var User = ds.define('User', {name: String}, {relations: {posts: {type: 'embedsMany', model: 'Post'}}}); + + assert(User.relations['posts']); + done(); + }); it('should set up foreign key with the correct type', function (done) { var ds = new DataSource('memory'); From dd9ea68f470e51d42989bb7b64a3890dbd9fd6a7 Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Mon, 11 Aug 2014 15:33:41 +0200 Subject: [PATCH 05/34] Refactored embedsMany (relationName vs. propertyName) --- lib/relation-definition.js | 94 ++++++++++++++++++++------------------ test/loopback-dl.test.js | 2 +- 2 files changed, 50 insertions(+), 46 deletions(-) diff --git a/lib/relation-definition.js b/lib/relation-definition.js index 2e8b1dc7..7b90c18b 100644 --- a/lib/relation-definition.js +++ b/lib/relation-definition.js @@ -9,6 +9,7 @@ var mergeQuery = require('./scope.js').mergeQuery; var ModelBaseClass = require('./model.js'); var applyFilter = require('./connectors/memory').applyFilter; var ValidationError = require('./validations.js').ValidationError; +var debug = require('debug')('loopback:relations'); exports.Relation = Relation; exports.RelationDefinition = RelationDefinition; @@ -89,7 +90,6 @@ function RelationDefinition(definition) { } definition = definition || {}; this.name = definition.name; - this.accessor = definition.accessor || this.name; assert(this.name, 'Relation name is missing'); this.type = normalizeType(definition.type); assert(this.type, 'Invalid relation type: ' + definition.type); @@ -1467,18 +1467,22 @@ RelationDefinition.embedsMany = function embedsMany(modelFrom, modelTo, params) modelTo = lookupModelTo(modelFrom, modelTo, params, true); var thisClassName = modelFrom.modelName; - var accessorName = params.as || (i8n.camelize(modelTo.modelName, true) + 'List'); - var relationName = params.property || i8n.camelize(modelTo.pluralModelName, true); - var fk = modelTo.dataSource.idName(modelTo.modelName) || 'id'; - var idName = modelFrom.dataSource.idName(modelFrom.modelName) || 'id'; + var relationName = params.as || (i8n.camelize(modelTo.modelName, true) + 'List'); + var propertyName = params.property || i8n.camelize(modelTo.pluralModelName, true); + var idName = modelTo.dataSource.idName(modelTo.modelName) || 'id'; - var definition = modelFrom.relations[accessorName] = new RelationDefinition({ - accessor: accessorName, + if (relationName === propertyName) { + propertyName = '_' + propertyName; + debug('EmbedsMany property cannot be equal to relation name: ' + + 'forcing property %s for relation %s', propertyName, relationName); + } + + var definition = modelFrom.relations[relationName] = new RelationDefinition({ name: relationName, type: RelationTypes.embedsMany, modelFrom: modelFrom, - keyFrom: idName, - keyTo: fk, + keyFrom: propertyName, + keyTo: idName, modelTo: modelTo, multiple: true, properties: params.properties, @@ -1487,7 +1491,7 @@ RelationDefinition.embedsMany = function embedsMany(modelFrom, modelTo, params) embed: true }); - modelFrom.dataSource.defineProperty(modelFrom.modelName, relationName, { + modelFrom.dataSource.defineProperty(modelFrom.modelName, propertyName, { type: [modelTo], default: function() { return []; } }); @@ -1495,14 +1499,14 @@ RelationDefinition.embedsMany = function embedsMany(modelFrom, modelTo, params) modelTo.validatesPresenceOf(idName); if (!params.polymorphic) { - modelFrom.validate(relationName, function(err) { - var embeddedList = this[relationName] || []; + modelFrom.validate(propertyName, function(err) { + var embeddedList = this[propertyName] || []; var ids = embeddedList.map(function(m) { return m[idName]; }); var uniqueIds = ids.filter(function(id, pos) { return ids.indexOf(id) === pos; }); if (ids.length !== uniqueIds.length) { - this.errors.add(relationName, 'Contains duplicate `' + idName + '`', 'uniqueness'); + this.errors.add(propertyName, 'Contains duplicate `' + idName + '`', 'uniqueness'); err(false); } }, { code: 'uniqueness' }) @@ -1510,9 +1514,9 @@ RelationDefinition.embedsMany = function embedsMany(modelFrom, modelTo, params) // validate all embedded items if (definition.options.validate) { - modelFrom.validate(relationName, function(err) { + modelFrom.validate(propertyName, function(err) { var self = this; - var embeddedList = this[relationName] || []; + var embeddedList = this[propertyName] || []; var hasErrors = false; embeddedList.forEach(function(item) { if (item instanceof modelTo) { @@ -1522,11 +1526,11 @@ RelationDefinition.embedsMany = function embedsMany(modelFrom, modelTo, params) var first = Object.keys(item.errors)[0]; var msg = 'contains invalid item: `' + id + '`'; msg += ' (' + first + ' ' + item.errors[first] + ')'; - self.errors.add(relationName, msg, 'invalid'); + self.errors.add(propertyName, msg, 'invalid'); } } else { hasErrors = true; - self.errors.add(relationName, 'Contains invalid item', 'invalid'); + self.errors.add(propertyName, 'Contains invalid item', 'invalid'); } }); if (hasErrors) err(false); @@ -1547,19 +1551,19 @@ RelationDefinition.embedsMany = function embedsMany(modelFrom, modelTo, params) }; var findByIdFunc = scopeMethods.findById; - modelFrom.prototype['__findById__' + accessorName] = findByIdFunc; + modelFrom.prototype['__findById__' + relationName] = findByIdFunc; var destroyByIdFunc = scopeMethods.destroy; - modelFrom.prototype['__destroyById__' + accessorName] = destroyByIdFunc; + modelFrom.prototype['__destroyById__' + relationName] = destroyByIdFunc; var updateByIdFunc = scopeMethods.updateById; - modelFrom.prototype['__updateById__' + accessorName] = updateByIdFunc; + modelFrom.prototype['__updateById__' + relationName] = updateByIdFunc; var addFunc = scopeMethods.add; - modelFrom.prototype['__link__' + accessorName] = addFunc; + modelFrom.prototype['__link__' + relationName] = addFunc; var removeFunc = scopeMethods.remove; - modelFrom.prototype['__unlink__' + accessorName] = removeFunc; + modelFrom.prototype['__unlink__' + relationName] = removeFunc; scopeMethods.create = scopeMethod(definition, 'create'); scopeMethods.build = scopeMethod(definition, 'build'); @@ -1572,12 +1576,12 @@ RelationDefinition.embedsMany = function embedsMany(modelFrom, modelTo, params) var methodName = customMethods[i]; var method = scopeMethods[methodName]; if (typeof method === 'function' && method.shared === true) { - modelFrom.prototype['__' + methodName + '__' + accessorName] = method; + modelFrom.prototype['__' + methodName + '__' + relationName] = method; } }; // Mix the property and scoped methods into the prototype class - var scopeDefinition = defineScope(modelFrom.prototype, modelTo, accessorName, function () { + var scopeDefinition = defineScope(modelFrom.prototype, modelTo, relationName, function () { return {}; }, scopeMethods, definition.options); @@ -1586,7 +1590,7 @@ RelationDefinition.embedsMany = function embedsMany(modelFrom, modelTo, params) EmbedsMany.prototype.related = function(receiver, scopeParams, condOrRefresh, cb) { var modelTo = this.definition.modelTo; - var relationName = this.definition.name; + var propertyName = this.definition.keyFrom; var modelInstance = this.modelInstance; var self = receiver; @@ -1605,7 +1609,7 @@ EmbedsMany.prototype.related = function(receiver, scopeParams, condOrRefresh, cb throw new Error('Method can be only called with one or two arguments'); } - var embeddedList = self[relationName] || []; + var embeddedList = self[propertyName] || []; this.definition.applyScope(modelInstance, actualCond); @@ -1627,12 +1631,12 @@ EmbedsMany.prototype.related = function(receiver, scopeParams, condOrRefresh, cb }; EmbedsMany.prototype.findById = function (fkId, cb) { - var pk = this.definition.keyFrom; + var pk = this.definition.keyTo; var modelTo = this.definition.modelTo; - var relationName = this.definition.name; + var propertyName = this.definition.keyFrom; var modelInstance = this.modelInstance; - var embeddedList = modelInstance[relationName] || []; + var embeddedList = modelInstance[propertyName] || []; var find = function(id) { for (var i = 0; i < embeddedList.length; i++) { @@ -1669,10 +1673,10 @@ EmbedsMany.prototype.updateById = function (fkId, data, cb) { } var modelTo = this.definition.modelTo; - var relationName = this.definition.name; + var propertyName = this.definition.keyFrom; var modelInstance = this.modelInstance; - var embeddedList = modelInstance[relationName] || []; + var embeddedList = modelInstance[propertyName] || []; var inst = this.findById(fkId); @@ -1690,7 +1694,7 @@ EmbedsMany.prototype.updateById = function (fkId, data, cb) { } if (typeof cb === 'function') { - modelInstance.updateAttribute(relationName, + modelInstance.updateAttribute(propertyName, embeddedList, function(err) { cb(err, inst); }); @@ -1705,10 +1709,10 @@ EmbedsMany.prototype.updateById = function (fkId, data, cb) { EmbedsMany.prototype.destroyById = function (fkId, cb) { var modelTo = this.definition.modelTo; - var relationName = this.definition.name; + var propertyName = this.definition.keyFrom; var modelInstance = this.modelInstance; - var embeddedList = modelInstance[relationName] || []; + var embeddedList = modelInstance[propertyName] || []; var inst = (fkId instanceof modelTo) ? fkId : this.findById(fkId); @@ -1716,7 +1720,7 @@ EmbedsMany.prototype.destroyById = function (fkId, cb) { var index = embeddedList.indexOf(inst); if (index > -1) embeddedList.splice(index, 1); if (typeof cb === 'function') { - modelInstance.updateAttribute(relationName, + modelInstance.updateAttribute(propertyName, embeddedList, function(err) { cb(err); }); @@ -1733,10 +1737,10 @@ EmbedsMany.prototype.unset = EmbedsMany.prototype.destroyById; EmbedsMany.prototype.at = function (index, cb) { var modelTo = this.definition.modelTo; - var relationName = this.definition.name; + var propertyName = this.definition.keyFrom; var modelInstance = this.modelInstance; - var embeddedList = modelInstance[relationName] || []; + var embeddedList = modelInstance[propertyName] || []; var item = embeddedList[parseInt(index)]; item = (item instanceof modelTo) ? item : null; @@ -1751,9 +1755,9 @@ EmbedsMany.prototype.at = function (index, cb) { }; EmbedsMany.prototype.create = function (targetModelData, cb) { - var pk = this.definition.keyFrom; + var pk = this.definition.keyTo; var modelTo = this.definition.modelTo; - var relationName = this.definition.name; + var propertyName = this.definition.keyFrom; var modelInstance = this.modelInstance; var autoId = this.definition.options.autoId !== false; @@ -1763,7 +1767,7 @@ EmbedsMany.prototype.create = function (targetModelData, cb) { } targetModelData = targetModelData || {}; - var embeddedList = modelInstance[relationName] || []; + var embeddedList = modelInstance[propertyName] || []; var inst = this.build(targetModelData); @@ -1775,20 +1779,20 @@ EmbedsMany.prototype.create = function (targetModelData, cb) { }); } - modelInstance.updateAttribute(relationName, + modelInstance.updateAttribute(propertyName, embeddedList, function(err, modelInst) { cb(err, err ? null : inst); }); }; EmbedsMany.prototype.build = function(targetModelData) { - var pk = this.definition.keyFrom; + var pk = this.definition.keyTo; var modelTo = this.definition.modelTo; - var relationName = this.definition.name; + var propertyName = this.definition.keyFrom; var modelInstance = this.modelInstance; var autoId = this.definition.options.autoId !== false; - var embeddedList = modelInstance[relationName] || []; + var embeddedList = modelInstance[propertyName] || []; targetModelData = targetModelData || {}; @@ -1890,7 +1894,7 @@ EmbedsMany.prototype.remove = function (acInst, cb) { belongsTo.applyScope(modelInstance, filter); - modelInstance[definition.accessor](filter, function(err, items) { + modelInstance[definition.name](filter, function(err, items) { if (err) return cb(err); items.forEach(function(item) { diff --git a/test/loopback-dl.test.js b/test/loopback-dl.test.js index f388e50e..458f91db 100644 --- a/test/loopback-dl.test.js +++ b/test/loopback-dl.test.js @@ -918,7 +918,7 @@ describe('Load models with relations', function () { var ds = new DataSource('memory'); var Post = ds.define('Post', {userId: Number, content: String}); - var User = ds.define('User', {name: String}, {relations: {posts: {type: 'embedsMany', model: 'Post'}}}); + var User = ds.define('User', {name: String}, {relations: {posts: {type: 'embedsMany', model: 'Post' }}}); assert(User.relations['posts']); done(); From 3b398c5f778c08bf584fbbe5b5a123db4338638f Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Mon, 11 Aug 2014 19:07:48 +0200 Subject: [PATCH 06/34] Fix scopeMethods closure issue --- lib/relation-definition.js | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/relation-definition.js b/lib/relation-definition.js index 7b90c18b..b9c298f8 100644 --- a/lib/relation-definition.js +++ b/lib/relation-definition.js @@ -64,12 +64,15 @@ function extendScopeMethods(definition, scopeMethods, ext) { if (typeof ext === 'function') { customMethods = ext.call(definition, scopeMethods, relationClass); } else if (typeof ext === 'object') { + function createFunc(definition, relationMethod) { + return function() { + var relation = new relationClass(definition, this); + return relationMethod.apply(relation, arguments); + }; + }; for (var key in ext) { var relationMethod = ext[key]; - var method = scopeMethods[key] = function () { - var relation = new relationClass(definition, this); - return relationMethod.apply(relation, arguments); - }; + var method = scopeMethods[key] = createFunc(definition, relationMethod); if (relationMethod.shared) { sharedMethod(definition, key, method, relationMethod); } From 4cb22793e2a3a19662fc06cbf4484ae76be787f2 Mon Sep 17 00:00:00 2001 From: Rand McKinney Date: Mon, 11 Aug 2014 11:42:21 -0700 Subject: [PATCH 07/34] Fix links to confluence docs --- lib/model-builder.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/model-builder.js b/lib/model-builder.js index 73936be5..708d328b 100644 --- a/lib/model-builder.js +++ b/lib/model-builder.js @@ -501,9 +501,9 @@ ModelBuilder.prototype.defineValueType = function(type, aliases) { * * @param {String} model Name of model * @options {Object} properties JSON object specifying properties. Each property is a key whos value is - * either the [type](http://docs.strongloop.com/display/DOC/LDL+data+types) or `propertyName: {options}` + * either the [type](http://docs.strongloop.com/display/LB/LoopBack+types) or `propertyName: {options}` * where the options are described below. - * @property {String} type Datatype of property: Must be an [LDL type](http://docs.strongloop.com/display/DOC/LDL+data+types). + * @property {String} type Datatype of property: Must be an [LDL type](http://docs.strongloop.com/display/LB/LoopBack+types). * @property {Boolean} index True if the property is an index; false otherwise. */ ModelBuilder.prototype.extendModel = function (model, props) { From 0d44cdc57309ca022f9bf22a52115e7cd2679b9d Mon Sep 17 00:00:00 2001 From: Jaka Hudoklin Date: Tue, 12 Aug 2014 14:44:33 +0200 Subject: [PATCH 08/34] add count to relations Signed-off-by: Jaka Hudoklin --- lib/scope.js | 13 +++++++++++++ test/relations.test.js | 10 ++++++++++ 2 files changed, 23 insertions(+) diff --git a/lib/scope.js b/lib/scope.js index 0c5ea86c..8f85ec7e 100644 --- a/lib/scope.js +++ b/lib/scope.js @@ -138,6 +138,7 @@ function defineScope(cls, targetClass, name, params, methods, options) { f.build = build; f.create = create; f.destroyAll = destroyAll; + f.count = count; for (var i in definition.methods) { f[i] = definition.methods[i].bind(self); } @@ -183,6 +184,13 @@ function defineScope(cls, targetClass, name, params, methods, options) { cls['__delete__' + name] = fn_delete; + var fn_count = function (cb) { + var f = this[name].count; + f.apply(this[name], arguments); + }; + + cls['__count__' + name] = fn_count; + /* * Extracting fixed property values for the scope from the where clause into * the data object @@ -239,6 +247,11 @@ function defineScope(cls, targetClass, name, params, methods, options) { var where = (this._scope && this._scope.where) || {}; targetClass.destroyAll(where, cb); } + + function count(cb) { + var where = (this._scope && this._scope.where) || {}; + targetClass.count(where, cb); + } return definition; } diff --git a/test/relations.test.js b/test/relations.test.js index d1a2003a..a93b24f4 100644 --- a/test/relations.test.js +++ b/test/relations.test.js @@ -537,6 +537,16 @@ describe('relations', function () { }); }); }); + + it('should find count of records on scope - scoped', function (done) { + Category.findOne(function (err, c) { + c.productType = 'tool'; // temporary, for scoping + c.products.count(function(err, count) { + count.should.equal(1); + done(); + }); + }); + }); it('should delete records on scope - scoped', function (done) { Category.findOne(function (err, c) { From 29e4314ec02428236e249d8df0d392b0e8888601 Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Wed, 13 Aug 2014 10:10:34 +0200 Subject: [PATCH 09/34] Fix formatting --- lib/relation-definition.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/relation-definition.js b/lib/relation-definition.js index b9c298f8..cfaaa1c1 100644 --- a/lib/relation-definition.js +++ b/lib/relation-definition.js @@ -65,10 +65,10 @@ function extendScopeMethods(definition, scopeMethods, ext) { customMethods = ext.call(definition, scopeMethods, relationClass); } else if (typeof ext === 'object') { function createFunc(definition, relationMethod) { - return function() { - var relation = new relationClass(definition, this); - return relationMethod.apply(relation, arguments); - }; + return function() { + var relation = new relationClass(definition, this); + return relationMethod.apply(relation, arguments); + }; }; for (var key in ext) { var relationMethod = ext[key]; From 710ad35b397a09fc84de9ed7dd7d3717c78a714f Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Wed, 13 Aug 2014 11:28:23 +0200 Subject: [PATCH 10/34] Implement scope.defineMethod/relation.defineMethod --- lib/relation-definition.js | 69 ++++++++++++++++++++++++++++------- lib/relations.js | 12 +++---- lib/scope.js | 9 +++++ test/relations.test.js | 73 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 144 insertions(+), 19 deletions(-) diff --git a/lib/relation-definition.js b/lib/relation-definition.js index 1171b998..592b311e 100644 --- a/lib/relation-definition.js +++ b/lib/relation-definition.js @@ -63,12 +63,15 @@ function extendScopeMethods(definition, scopeMethods, ext) { if (typeof ext === 'function') { customMethods = ext.call(definition, scopeMethods, relationClass); } else if (typeof ext === 'object') { - for (var key in ext) { - var relationMethod = ext[key]; - var method = scopeMethods[key] = function () { + function createFunc(definition, relationMethod) { + return function() { var relation = new relationClass(definition, this); return relationMethod.apply(relation, arguments); }; + }; + for (var key in ext) { + var relationMethod = ext[key]; + var method = scopeMethods[key] = createFunc(definition, relationMethod); if (relationMethod.shared) { sharedMethod(definition, key, method, relationMethod); } @@ -128,6 +131,29 @@ RelationDefinition.prototype.toJSON = function () { return json; }; +/** + * Define a relation scope method + * @param {String} name of the method + * @param {Function} function to define + */ +RelationDefinition.prototype.defineMethod = function(name, fn) { + var relationClass = RelationClasses[this.type]; + var relationName = this.name; + var modelFrom = this.modelFrom; + var definition = this; + var scope = this.modelFrom.scopes[this.name]; + if (!scope) throw new Error('Unknown relation scope: ' + this.name); + var method = scope.defineMethod(name, function() { + var relation = new relationClass(definition, this); + return fn.apply(relation, arguments); + }); + if (fn.shared) { + sharedMethod(definition, name, method, fn); + modelFrom.prototype['__' + name + '__' + relationName] = method; + } + return method; +}; + /** * Apply the configured scope to the filter/query object. * @param {Object} modelInstance @@ -201,6 +227,15 @@ Relation.prototype.getCache = function () { return this.modelInstance.__cachedRelations[this.definition.name]; }; +/** + * Fetch the related model(s) - this is a helper method to unify access. + * @param (Boolean|Object} condOrRefresh refresh or conditions object + * @param {Function} cb callback + */ +Relation.prototype.fetch = function(condOrRefresh, cb) { + this.modelInstance[this.definition.name].apply(this.modelInstance, arguments); +}; + /** * HasMany subclass * @param {RelationDefinition|Object} definition @@ -554,7 +589,8 @@ RelationDefinition.hasMany = function hasMany(modelFrom, modelTo, params) { return filter; }, scopeMethods, definition.options); - + + return definition; }; function scopeMethod(definition, methodName) { @@ -993,7 +1029,7 @@ RelationDefinition.belongsTo = function (modelFrom, modelTo, params) { modelFrom.dataSource.defineForeignKey(modelFrom.modelName, fk, modelTo.modelName); } - var relationDef = modelFrom.relations[relationName] = new RelationDefinition({ + var definition = modelFrom.relations[relationName] = new RelationDefinition({ name: relationName, type: RelationTypes.belongsTo, modelFrom: modelFrom, @@ -1011,12 +1047,12 @@ RelationDefinition.belongsTo = function (modelFrom, modelTo, params) { enumerable: true, configurable: true, get: function() { - var relation = new BelongsTo(relationDef, this); + var relation = new BelongsTo(definition, this); var relationMethod = relation.related.bind(relation); relationMethod.create = relation.create.bind(relation); relationMethod.build = relation.build.bind(relation); - if (relationDef.modelTo) { - relationMethod._targetClass = relationDef.modelTo.modelName; + if (definition.modelTo) { + relationMethod._targetClass = definition.modelTo.modelName; } return relationMethod; } @@ -1030,6 +1066,8 @@ RelationDefinition.belongsTo = function (modelFrom, modelTo, params) { f.apply(this, arguments); }; modelFrom.prototype['__get__' + relationName] = fn; + + return definition; }; BelongsTo.prototype.create = function(targetModelData, cb) { @@ -1222,8 +1260,7 @@ RelationDefinition.hasAndBelongsToMany = function hasAndBelongsToMany(modelFrom, params.through.belongsTo(modelTo); - this.hasMany(modelFrom, modelTo, options); - + return this.hasMany(modelFrom, modelTo, options); }; /** @@ -1268,7 +1305,7 @@ RelationDefinition.hasOne = function (modelFrom, modelTo, params) { } } - var relationDef = modelFrom.relations[relationName] = new RelationDefinition({ + var definition = modelFrom.relations[relationName] = new RelationDefinition({ name: relationName, type: RelationTypes.hasOne, modelFrom: modelFrom, @@ -1287,11 +1324,11 @@ RelationDefinition.hasOne = function (modelFrom, modelTo, params) { enumerable: true, configurable: true, get: function() { - var relation = new HasOne(relationDef, this); + var relation = new HasOne(definition, this); var relationMethod = relation.related.bind(relation) relationMethod.create = relation.create.bind(relation); relationMethod.build = relation.build.bind(relation); - relationMethod._targetClass = relationDef.modelTo.modelName; + relationMethod._targetClass = definition.modelTo.modelName; return relationMethod; } }); @@ -1304,6 +1341,8 @@ RelationDefinition.hasOne = function (modelFrom, modelTo, params) { f.apply(this, arguments); }; modelFrom.prototype['__get__' + relationName] = fn; + + return definition; }; /** @@ -1603,6 +1642,8 @@ RelationDefinition.embedsMany = function embedsMany(modelFrom, modelTo, params) }, scopeMethods, definition.options); scopeDefinition.related = scopeMethods.related; + + return definition; }; EmbedsMany.prototype.related = function(receiver, scopeParams, condOrRefresh, cb) { @@ -2017,6 +2058,8 @@ RelationDefinition.referencesMany = function referencesMany(modelFrom, modelTo, }, scopeMethods, definition.options); scopeDefinition.related = scopeMethods.related; // bound to definition + + return definition; }; ReferencesMany.prototype.related = function(receiver, scopeParams, condOrRefresh, cb) { diff --git a/lib/relations.js b/lib/relations.js index d3e735ff..ba9a36d7 100644 --- a/lib/relations.js +++ b/lib/relations.js @@ -65,7 +65,7 @@ function RelationMixin() { * @property {Object} model Model object */ RelationMixin.hasMany = function hasMany(modelTo, params) { - RelationDefinition.hasMany(this, modelTo, params); + return RelationDefinition.hasMany(this, modelTo, params); }; /** @@ -120,7 +120,7 @@ RelationMixin.hasMany = function hasMany(modelTo, params) { * */ RelationMixin.belongsTo = function (modelTo, params) { - RelationDefinition.belongsTo(this, modelTo, params); + return RelationDefinition.belongsTo(this, modelTo, params); }; /** @@ -155,17 +155,17 @@ RelationMixin.belongsTo = function (modelTo, params) { * @property {Object} model Model object */ RelationMixin.hasAndBelongsToMany = function hasAndBelongsToMany(modelTo, params) { - RelationDefinition.hasAndBelongsToMany(this, modelTo, params); + return RelationDefinition.hasAndBelongsToMany(this, modelTo, params); }; RelationMixin.hasOne = function hasMany(modelTo, params) { - RelationDefinition.hasOne(this, modelTo, params); + return RelationDefinition.hasOne(this, modelTo, params); }; RelationMixin.referencesMany = function hasMany(modelTo, params) { - RelationDefinition.referencesMany(this, modelTo, params); + return RelationDefinition.referencesMany(this, modelTo, params); }; RelationMixin.embedsMany = function hasMany(modelTo, params) { - RelationDefinition.embedsMany(this, modelTo, params); + return RelationDefinition.embedsMany(this, modelTo, params); }; diff --git a/lib/scope.js b/lib/scope.js index 0c5ea86c..a95a3925 100644 --- a/lib/scope.js +++ b/lib/scope.js @@ -55,6 +55,15 @@ ScopeDefinition.prototype.related = function(receiver, scopeParams, condOrRefres } } +/** + * Define a scope method + * @param {String} name of the method + * @param {Function} function to define + */ +ScopeDefinition.prototype.defineMethod = function(name, fn) { + return this.methods[name] = fn; +} + /** * Define a scope to the class * @param {Model} cls The class where the scope method is added diff --git a/test/relations.test.js b/test/relations.test.js index d1a2003a..ce0e62f3 100644 --- a/test/relations.test.js +++ b/test/relations.test.js @@ -2129,5 +2129,78 @@ describe('relations', function () { }); }); + + describe('custom relation/scope methods', function () { + + before(function (done) { + db = getSchema(); + Category = db.define('Category', {name: String}); + Product = db.define('Product', {name: String}); + + db.automigrate(function () { + Category.destroyAll(function() { + Product.destroyAll(done); + }); + }); + }); + + it('can be declared', function (done) { + var relation = Category.hasMany(Product); + + var summarize = function(cb) { + var modelInstance = this.modelInstance; + this.fetch(function(err, items) { + if (err) return cb(err, []); + var summary = items.map(function(item) { + var obj = item.toObject(); + obj.categoryName = modelInstance.name; + return obj; + }); + cb(null, summary); + }); + }; + + summarize.shared = true; // remoting + summarize.http = { verb: 'get', path: '/products/summary' }; + + relation.defineMethod('summarize', summarize); + + Category.prototype['__summarize__products'].should.be.a.function; + should.exist(Category.prototype['__summarize__products'].shared); + Category.prototype['__summarize__products'].http.should.eql(summarize.http); + + db.automigrate(done); + }); + + it('should setup test records', function (done) { + Category.create({ name: 'Category A' }, function(err, cat) { + cat.products.create({ name: 'Product 1' }, function(err, p) { + cat.products.create({ name: 'Product 2' }, function(err, p) { + done(); + }); + }) + }); + }); + + it('should allow custom scope methods - summarize', function(done) { + var expected = [ + { name: 'Product 1', categoryId: 1, categoryName: 'Category A' }, + { name: 'Product 2', categoryId: 1, categoryName: 'Category A' } + ]; + + Category.findOne(function(err, cat) { + cat.products.summarize(function(err, summary) { + should.not.exist(err); + var result = summary.map(function(item) { + delete item.id; + return item; + }); + result.should.eql(expected); + done(); + }); + }) + }); + + }); }); From d7555bfb64bf1923a4c65cb6d86d301618aa6ded Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Wed, 13 Aug 2014 16:24:11 +0200 Subject: [PATCH 11/34] Allow runtime override of scope/relation order query param --- lib/scope.js | 6 +++--- test/scope.test.js | 54 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 3 deletions(-) diff --git a/lib/scope.js b/lib/scope.js index 0c5ea86c..dd0fe194 100644 --- a/lib/scope.js +++ b/lib/scope.js @@ -277,11 +277,11 @@ function mergeQuery(base, update) { base.collect = update.collect; } - // overwrite order - if (update.order) { + // set order + if (!base.order && update.order) { base.order = update.order; } - + // overwrite pagination if (update.limit !== undefined) { base.limit = update.limit; diff --git a/test/scope.test.js b/test/scope.test.js index 2060c80e..c8033c82 100644 --- a/test/scope.test.js +++ b/test/scope.test.js @@ -76,3 +76,57 @@ describe('scope', function () { }); }); }); + +describe('scope - order', function () { + + before(function () { + db = getSchema(); + Station = db.define('Station', { + name: {type: String, index: true}, + order: {type: Number, index: true} + }); + Station.scope('reverse', {order: 'order DESC'}); + }); + + beforeEach(function (done) { + Station.destroyAll(done); + }); + + beforeEach(function (done) { + Station.create({ name: 'a', order: 1 }, done); + }); + + beforeEach(function (done) { + Station.create({ name: 'b', order: 2 }, done); + }); + + beforeEach(function (done) { + Station.create({ name: 'c', order: 3 }, done); + }); + + it('should define scope with default order', function (done) { + Station.reverse(function(err, stations) { + stations[0].name.should.equal('c'); + stations[0].order.should.equal(3); + stations[1].name.should.equal('b'); + stations[1].order.should.equal(2); + stations[2].name.should.equal('a'); + stations[2].order.should.equal(1); + done(); + }); + }); + + it('should override default scope order', function (done) { + Station.reverse({order: 'order ASC'}, function(err, stations) { + stations[0].name.should.equal('a'); + stations[0].order.should.equal(1); + stations[1].name.should.equal('b'); + stations[1].order.should.equal(2); + stations[2].name.should.equal('c'); + stations[2].order.should.equal(3); + done(); + }); + }); + +}); + From f2025c3995f542b8679e0261346e6271f667a4ef Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Thu, 14 Aug 2014 09:40:03 +0200 Subject: [PATCH 12/34] Allow partial list of ids for sortByIds --- lib/dao.js | 32 +++++++++++++++++++++----------- test/basic-querying.test.js | 27 +++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 11 deletions(-) diff --git a/lib/dao.js b/lib/dao.js index 4ed71b3e..fa00aa4c 100644 --- a/lib/dao.js +++ b/lib/dao.js @@ -351,19 +351,29 @@ DataAccessObject.sortByIds = function(ids, results) { return (typeof id === 'object') ? id.toString() : id; }); - results.sort(function(x, y) { - var idA = (typeof x[pk] === 'object') ? x[pk].toString() : x[pk]; - var idB = (typeof y[pk] === 'object') ? y[pk].toString() : y[pk]; - var a = ids.indexOf(idA); - var b = ids.indexOf(idB); - if (a === -1 || b === -1) return 1; // last - if (a !== b) { - if (a > b) return 1; - if (a < b) return -1; - } + var indexOf = function(x) { + var id = (typeof x[pk] === 'object') ? x[pk].toString() : x[pk]; + return ids.indexOf(id); + }; + + var heading = []; + var tailing = []; + + results.forEach(function(x) { + var idx = indexOf(x); + idx === -1 ? tailing.push(x) : heading.push(x); }); - return results; + heading.sort(function(x, y) { + var a = indexOf(x); + var b = indexOf(y); + if (a === -1 || b === -1) return 1; // last + if (a === b) return 0; + if (a > b) return 1; + if (a < b) return -1; + }); + + return heading.concat(tailing); }; function convertNullToNotFoundError(ctx, cb) { diff --git a/test/basic-querying.test.js b/test/basic-querying.test.js index dd26928a..d978fe88 100644 --- a/test/basic-querying.test.js +++ b/test/basic-querying.test.js @@ -87,6 +87,33 @@ describe('basic-querying', function () { done(); }); }); + + it('should sortByIds', function(done) { + User.find(function(err, users) { + sorted = User.sortByIds([6, 5, 4, 3, 2, 1], users); + var names = sorted.map(function(u) { return u.name; }); + names.should.eql(['f', 'e', 'd', 'c', 'b', 'a']); + done(); + }); + }); + + it('should sortByIds - partial ids (1)', function(done) { + User.find(function(err, users) { + sorted = User.sortByIds([6, 5, 4], users); + var names = sorted.map(function(u) { return u.name; }); + names.should.eql(['f', 'e', 'd', 'a', 'b', 'c']); + done(); + }); + }); + + it('should sortByIds - partial ids (2)', function(done) { + User.find(function(err, users) { + sorted = User.sortByIds([5, 3, 2], users); + var names = sorted.map(function(u) { return u.name; }); + names.should.eql(['e', 'c', 'b', 'a', 'd', 'f']); + done(); + }); + }); }); From e6c9de02e2439a1af7c5620928cbb914a6834550 Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Thu, 14 Aug 2014 17:13:58 +0200 Subject: [PATCH 13/34] Remove redundant test --- test/basic-querying.test.js | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/test/basic-querying.test.js b/test/basic-querying.test.js index d978fe88..184335e1 100644 --- a/test/basic-querying.test.js +++ b/test/basic-querying.test.js @@ -97,16 +97,7 @@ describe('basic-querying', function () { }); }); - it('should sortByIds - partial ids (1)', function(done) { - User.find(function(err, users) { - sorted = User.sortByIds([6, 5, 4], users); - var names = sorted.map(function(u) { return u.name; }); - names.should.eql(['f', 'e', 'd', 'a', 'b', 'c']); - done(); - }); - }); - - it('should sortByIds - partial ids (2)', function(done) { + it('should sortByIds - partial ids', function(done) { User.find(function(err, users) { sorted = User.sortByIds([5, 3, 2], users); var names = sorted.map(function(u) { return u.name; }); From 807a6aaf3f7f9baf80da0314210a48dac7dff766 Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Sat, 9 Aug 2014 10:58:03 +0200 Subject: [PATCH 14/34] Refactor modelTo logic into lookupModelTo --- lib/relation-definition.js | 81 +++++++++++--------------------------- test/mixins.test.js | 12 ++++-- 2 files changed, 32 insertions(+), 61 deletions(-) diff --git a/lib/relation-definition.js b/lib/relation-definition.js index 15d78da6..011b545b 100644 --- a/lib/relation-definition.js +++ b/lib/relation-definition.js @@ -398,6 +398,21 @@ function lookupModel(models, modelName) { } } +function lookupModelTo(modelFrom, modelTo, params, singularize) { + if ('string' === typeof modelTo) { + params.as = params.as || modelTo; + modelTo = params.model || modelTo; + if (typeof modelTo === 'string') { + var modelToName = (singularize ? i8n.singularize(modelTo) : modelTo).toLowerCase(); + modelTo = lookupModel(modelFrom.dataSource.modelBuilder.models, modelToName) || modelTo; + } + if (typeof modelTo !== 'function') { + throw new Error('Could not find "' + modelTo + '" relation for ' + modelFrom.modelName); + } + } + return modelTo; +} + /*! * Normalize polymorphic parameters * @param {Object|String} params Name of the polymorphic relation or params @@ -436,15 +451,7 @@ function polymorphicParams(params) { RelationDefinition.hasMany = function hasMany(modelFrom, modelTo, params) { var thisClassName = modelFrom.modelName; params = params || {}; - if (typeof modelTo === 'string') { - params.as = modelTo; - if (params.model) { - modelTo = params.model; - } else { - var modelToName = i8n.singularize(modelTo).toLowerCase(); - modelTo = lookupModel(modelFrom.dataSource.modelBuilder.models, modelToName); - } - } + modelTo = lookupModelTo(modelFrom, modelTo, params, true); var relationName = params.as || i8n.camelize(modelTo.pluralModelName, true); var fk = params.foreignKey || i8n.camelize(thisClassName + '_id', true); @@ -953,13 +960,7 @@ RelationDefinition.belongsTo = function (modelFrom, modelTo, params) { var discriminator, polymorphic; params = params || {}; if ('string' === typeof modelTo && !params.polymorphic) { - params.as = modelTo; - if (params.model) { - modelTo = params.model; - } else { - var modelToName = modelTo.toLowerCase(); - modelTo = lookupModel(modelFrom.dataSource.modelBuilder.models, modelToName); - } + modelTo = lookupModelTo(modelFrom, modelTo, params); } var idName, relationName, fk; @@ -1182,19 +1183,9 @@ BelongsTo.prototype.related = function (refresh, params) { */ RelationDefinition.hasAndBelongsToMany = function hasAndBelongsToMany(modelFrom, modelTo, params) { params = params || {}; + modelTo = lookupModelTo(modelFrom, modelTo, params, true); + var models = modelFrom.dataSource.modelBuilder.models; - - if ('string' === typeof modelTo) { - params.as = modelTo; - if (params.model) { - modelTo = params.model; - } else { - modelTo = lookupModel(models, i8n.singularize(modelTo)) || modelTo; - } - if (typeof modelTo === 'string') { - throw new Error('Could not find "' + modelTo + '" relation for ' + modelFrom.modelName); - } - } if (!params.through) { if (params.polymorphic) throw new Error('Polymorphic relations need a through model'); @@ -1243,15 +1234,7 @@ RelationDefinition.hasAndBelongsToMany = function hasAndBelongsToMany(modelFrom, */ RelationDefinition.hasOne = function (modelFrom, modelTo, params) { params = params || {}; - if ('string' === typeof modelTo) { - params.as = modelTo; - if (params.model) { - modelTo = params.model; - } else { - var modelToName = modelTo.toLowerCase(); - modelTo = lookupModel(modelFrom.dataSource.modelBuilder.models, modelToName); - } - } + modelTo = lookupModelTo(modelFrom, modelTo, params); var pk = modelFrom.dataSource.idName(modelTo.modelName) || 'id'; var relationName = params.as || i8n.camelize(modelTo.modelName, true); @@ -1476,18 +1459,10 @@ HasOne.prototype.related = function (refresh, params) { }; RelationDefinition.embedsMany = function embedsMany(modelFrom, modelTo, params) { - var thisClassName = modelFrom.modelName; params = params || {}; - if (typeof modelTo === 'string') { - params.as = modelTo; - if (params.model) { - modelTo = params.model; - } else { - var modelToName = i8n.singularize(modelTo).toLowerCase(); - modelTo = lookupModel(modelFrom.dataSource.modelBuilder.models, modelToName); - } - } + modelTo = lookupModelTo(modelFrom, modelTo, params, true); + var thisClassName = modelFrom.modelName; var accessorName = params.as || (i8n.camelize(modelTo.modelName, true) + 'List'); var relationName = params.property || i8n.camelize(modelTo.pluralModelName, true); var fk = modelTo.dataSource.idName(modelTo.modelName) || 'id'; @@ -1923,18 +1898,10 @@ EmbedsMany.prototype.remove = function (acInst, cb) { }; RelationDefinition.referencesMany = function referencesMany(modelFrom, modelTo, params) { - var thisClassName = modelFrom.modelName; params = params || {}; - if (typeof modelTo === 'string') { - params.as = modelTo; - if (params.model) { - modelTo = params.model; - } else { - var modelToName = i8n.singularize(modelTo).toLowerCase(); - modelTo = lookupModel(modelFrom.dataSource.modelBuilder.models, modelToName); - } - } + modelTo = lookupModelTo(modelFrom, modelTo, params, true); + var thisClassName = modelFrom.modelName; var relationName = params.as || i8n.camelize(modelTo.pluralModelName, true); var fk = params.foreignKey || i8n.camelize(modelTo.modelName + '_ids', true); var idName = modelTo.dataSource.idName(modelTo.modelName) || 'id'; diff --git a/test/mixins.test.js b/test/mixins.test.js index f1dacf9d..e60f7e02 100644 --- a/test/mixins.test.js +++ b/test/mixins.test.js @@ -42,12 +42,15 @@ mixins.define('TimeStamp', timestamps); describe('Model class', function () { - it('should define a mixin', function() { + it('should define mixins', function() { mixins.define('Example', function(Model, options) { Model.prototype.example = function() { return options; }; }); + mixins.define('Demo', function(Model, options) { + Model.demoMixin = options.ok; + }); }); it('should apply a mixin class', function() { @@ -58,7 +61,7 @@ describe('Model class', function () { var memory = new DataSource('mem', {connector: Memory}, modelBuilder); var Item = memory.createModel('Item', { name: 'string' }, { - mixins: { TimeStamp: true, demo: true, Address: true } + mixins: { Address: true } }); var properties = Item.definition.properties; @@ -70,11 +73,12 @@ describe('Model class', function () { it('should apply mixins', function(done) { var memory = new DataSource('mem', {connector: Memory}, modelBuilder); var Item = memory.createModel('Item', { name: 'string' }, { - mixins: { TimeStamp: true, demo: { ok: true } } + mixins: { TimeStamp: true, Demo: { ok: true } } }); Item.mixin('Example', { foo: 'bar' }); - Item.mixin('other'); + + Item.demoMixin.should.be.true; var properties = Item.definition.properties; properties.createdAt.should.eql({ type: Date }); From a243d058809231106434c411a82321dd328ba5d3 Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Mon, 11 Aug 2014 14:43:51 +0200 Subject: [PATCH 15/34] Enable DL definition of embedsMany + referencesMany --- lib/datasource.js | 3 ++- test/loopback-dl.test.js | 20 ++++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/lib/datasource.js b/lib/datasource.js index b488ce72..9602ec4f 100644 --- a/lib/datasource.js +++ b/lib/datasource.js @@ -3,6 +3,7 @@ */ var ModelBuilder = require('./model-builder.js').ModelBuilder; var ModelDefinition = require('./model-definition.js'); +var RelationDefinition = require('./relation-definition.js'); var jutil = require('./jutil'); var utils = require('./utils'); var ModelBaseClass = require('./model.js'); @@ -364,7 +365,7 @@ function isModelClass(cls) { return cls.prototype instanceof ModelBaseClass; } -DataSource.relationTypes = ['belongsTo', 'hasMany', 'hasAndBelongsToMany', 'hasOne']; +DataSource.relationTypes = Object.keys(RelationDefinition.RelationTypes); function isModelDataSourceAttached(model) { return model && (!model.settings.unresolved) && (model.dataSource instanceof DataSource); diff --git a/test/loopback-dl.test.js b/test/loopback-dl.test.js index 18f0e0ad..f388e50e 100644 --- a/test/loopback-dl.test.js +++ b/test/loopback-dl.test.js @@ -903,6 +903,26 @@ describe('Load models with relations', function () { assert(Post.relations['user']); done(); }); + + it('should set up referencesMany relations', function (done) { + var ds = new DataSource('memory'); + + var Post = ds.define('Post', {userId: Number, content: String}); + var User = ds.define('User', {name: String}, {relations: {posts: {type: 'referencesMany', model: 'Post'}}}); + + assert(User.relations['posts']); + done(); + }); + + it('should set up embedsMany relations', function (done) { + var ds = new DataSource('memory'); + + var Post = ds.define('Post', {userId: Number, content: String}); + var User = ds.define('User', {name: String}, {relations: {posts: {type: 'embedsMany', model: 'Post'}}}); + + assert(User.relations['posts']); + done(); + }); it('should set up foreign key with the correct type', function (done) { var ds = new DataSource('memory'); From a67759dcbf7c0c7d8634cb443b398fc04442dc60 Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Mon, 11 Aug 2014 15:33:41 +0200 Subject: [PATCH 16/34] Refactored embedsMany (relationName vs. propertyName) --- lib/relation-definition.js | 94 ++++++++++++++++++++------------------ test/loopback-dl.test.js | 2 +- 2 files changed, 50 insertions(+), 46 deletions(-) diff --git a/lib/relation-definition.js b/lib/relation-definition.js index 011b545b..eeafd328 100644 --- a/lib/relation-definition.js +++ b/lib/relation-definition.js @@ -9,6 +9,7 @@ var mergeQuery = require('./scope.js').mergeQuery; var ModelBaseClass = require('./model.js'); var applyFilter = require('./connectors/memory').applyFilter; var ValidationError = require('./validations.js').ValidationError; +var debug = require('debug')('loopback:relations'); exports.Relation = Relation; exports.RelationDefinition = RelationDefinition; @@ -89,7 +90,6 @@ function RelationDefinition(definition) { } definition = definition || {}; this.name = definition.name; - this.accessor = definition.accessor || this.name; assert(this.name, 'Relation name is missing'); this.type = normalizeType(definition.type); assert(this.type, 'Invalid relation type: ' + definition.type); @@ -1463,18 +1463,22 @@ RelationDefinition.embedsMany = function embedsMany(modelFrom, modelTo, params) modelTo = lookupModelTo(modelFrom, modelTo, params, true); var thisClassName = modelFrom.modelName; - var accessorName = params.as || (i8n.camelize(modelTo.modelName, true) + 'List'); - var relationName = params.property || i8n.camelize(modelTo.pluralModelName, true); - var fk = modelTo.dataSource.idName(modelTo.modelName) || 'id'; - var idName = modelFrom.dataSource.idName(modelFrom.modelName) || 'id'; + var relationName = params.as || (i8n.camelize(modelTo.modelName, true) + 'List'); + var propertyName = params.property || i8n.camelize(modelTo.pluralModelName, true); + var idName = modelTo.dataSource.idName(modelTo.modelName) || 'id'; - var definition = modelFrom.relations[accessorName] = new RelationDefinition({ - accessor: accessorName, + if (relationName === propertyName) { + propertyName = '_' + propertyName; + debug('EmbedsMany property cannot be equal to relation name: ' + + 'forcing property %s for relation %s', propertyName, relationName); + } + + var definition = modelFrom.relations[relationName] = new RelationDefinition({ name: relationName, type: RelationTypes.embedsMany, modelFrom: modelFrom, - keyFrom: idName, - keyTo: fk, + keyFrom: propertyName, + keyTo: idName, modelTo: modelTo, multiple: true, properties: params.properties, @@ -1483,7 +1487,7 @@ RelationDefinition.embedsMany = function embedsMany(modelFrom, modelTo, params) embed: true }); - modelFrom.dataSource.defineProperty(modelFrom.modelName, relationName, { + modelFrom.dataSource.defineProperty(modelFrom.modelName, propertyName, { type: [modelTo], default: function() { return []; } }); @@ -1491,14 +1495,14 @@ RelationDefinition.embedsMany = function embedsMany(modelFrom, modelTo, params) modelTo.validatesPresenceOf(idName); if (!params.polymorphic) { - modelFrom.validate(relationName, function(err) { - var embeddedList = this[relationName] || []; + modelFrom.validate(propertyName, function(err) { + var embeddedList = this[propertyName] || []; var ids = embeddedList.map(function(m) { return m[idName]; }); var uniqueIds = ids.filter(function(id, pos) { return ids.indexOf(id) === pos; }); if (ids.length !== uniqueIds.length) { - this.errors.add(relationName, 'Contains duplicate `' + idName + '`', 'uniqueness'); + this.errors.add(propertyName, 'Contains duplicate `' + idName + '`', 'uniqueness'); err(false); } }, { code: 'uniqueness' }) @@ -1506,9 +1510,9 @@ RelationDefinition.embedsMany = function embedsMany(modelFrom, modelTo, params) // validate all embedded items if (definition.options.validate) { - modelFrom.validate(relationName, function(err) { + modelFrom.validate(propertyName, function(err) { var self = this; - var embeddedList = this[relationName] || []; + var embeddedList = this[propertyName] || []; var hasErrors = false; embeddedList.forEach(function(item) { if (item instanceof modelTo) { @@ -1518,11 +1522,11 @@ RelationDefinition.embedsMany = function embedsMany(modelFrom, modelTo, params) var first = Object.keys(item.errors)[0]; var msg = 'contains invalid item: `' + id + '`'; msg += ' (' + first + ' ' + item.errors[first] + ')'; - self.errors.add(relationName, msg, 'invalid'); + self.errors.add(propertyName, msg, 'invalid'); } } else { hasErrors = true; - self.errors.add(relationName, 'Contains invalid item', 'invalid'); + self.errors.add(propertyName, 'Contains invalid item', 'invalid'); } }); if (hasErrors) err(false); @@ -1543,19 +1547,19 @@ RelationDefinition.embedsMany = function embedsMany(modelFrom, modelTo, params) }; var findByIdFunc = scopeMethods.findById; - modelFrom.prototype['__findById__' + accessorName] = findByIdFunc; + modelFrom.prototype['__findById__' + relationName] = findByIdFunc; var destroyByIdFunc = scopeMethods.destroy; - modelFrom.prototype['__destroyById__' + accessorName] = destroyByIdFunc; + modelFrom.prototype['__destroyById__' + relationName] = destroyByIdFunc; var updateByIdFunc = scopeMethods.updateById; - modelFrom.prototype['__updateById__' + accessorName] = updateByIdFunc; + modelFrom.prototype['__updateById__' + relationName] = updateByIdFunc; var addFunc = scopeMethods.add; - modelFrom.prototype['__link__' + accessorName] = addFunc; + modelFrom.prototype['__link__' + relationName] = addFunc; var removeFunc = scopeMethods.remove; - modelFrom.prototype['__unlink__' + accessorName] = removeFunc; + modelFrom.prototype['__unlink__' + relationName] = removeFunc; scopeMethods.create = scopeMethod(definition, 'create'); scopeMethods.build = scopeMethod(definition, 'build'); @@ -1568,12 +1572,12 @@ RelationDefinition.embedsMany = function embedsMany(modelFrom, modelTo, params) var methodName = customMethods[i]; var method = scopeMethods[methodName]; if (typeof method === 'function' && method.shared === true) { - modelFrom.prototype['__' + methodName + '__' + accessorName] = method; + modelFrom.prototype['__' + methodName + '__' + relationName] = method; } }; // Mix the property and scoped methods into the prototype class - var scopeDefinition = defineScope(modelFrom.prototype, modelTo, accessorName, function () { + var scopeDefinition = defineScope(modelFrom.prototype, modelTo, relationName, function () { return {}; }, scopeMethods, definition.options); @@ -1582,7 +1586,7 @@ RelationDefinition.embedsMany = function embedsMany(modelFrom, modelTo, params) EmbedsMany.prototype.related = function(receiver, scopeParams, condOrRefresh, cb) { var modelTo = this.definition.modelTo; - var relationName = this.definition.name; + var propertyName = this.definition.keyFrom; var modelInstance = this.modelInstance; var self = receiver; @@ -1601,7 +1605,7 @@ EmbedsMany.prototype.related = function(receiver, scopeParams, condOrRefresh, cb throw new Error('Method can be only called with one or two arguments'); } - var embeddedList = self[relationName] || []; + var embeddedList = self[propertyName] || []; this.definition.applyScope(modelInstance, actualCond); @@ -1623,12 +1627,12 @@ EmbedsMany.prototype.related = function(receiver, scopeParams, condOrRefresh, cb }; EmbedsMany.prototype.findById = function (fkId, cb) { - var pk = this.definition.keyFrom; + var pk = this.definition.keyTo; var modelTo = this.definition.modelTo; - var relationName = this.definition.name; + var propertyName = this.definition.keyFrom; var modelInstance = this.modelInstance; - var embeddedList = modelInstance[relationName] || []; + var embeddedList = modelInstance[propertyName] || []; var find = function(id) { for (var i = 0; i < embeddedList.length; i++) { @@ -1665,10 +1669,10 @@ EmbedsMany.prototype.updateById = function (fkId, data, cb) { } var modelTo = this.definition.modelTo; - var relationName = this.definition.name; + var propertyName = this.definition.keyFrom; var modelInstance = this.modelInstance; - var embeddedList = modelInstance[relationName] || []; + var embeddedList = modelInstance[propertyName] || []; var inst = this.findById(fkId); @@ -1684,7 +1688,7 @@ EmbedsMany.prototype.updateById = function (fkId, data, cb) { } if (typeof cb === 'function') { - modelInstance.updateAttribute(relationName, + modelInstance.updateAttribute(propertyName, embeddedList, function(err) { cb(err, inst); }); @@ -1699,10 +1703,10 @@ EmbedsMany.prototype.updateById = function (fkId, data, cb) { EmbedsMany.prototype.destroyById = function (fkId, cb) { var modelTo = this.definition.modelTo; - var relationName = this.definition.name; + var propertyName = this.definition.keyFrom; var modelInstance = this.modelInstance; - var embeddedList = modelInstance[relationName] || []; + var embeddedList = modelInstance[propertyName] || []; var inst = (fkId instanceof modelTo) ? fkId : this.findById(fkId); @@ -1710,7 +1714,7 @@ EmbedsMany.prototype.destroyById = function (fkId, cb) { var index = embeddedList.indexOf(inst); if (index > -1) embeddedList.splice(index, 1); if (typeof cb === 'function') { - modelInstance.updateAttribute(relationName, + modelInstance.updateAttribute(propertyName, embeddedList, function(err) { cb(err); }); @@ -1727,10 +1731,10 @@ EmbedsMany.prototype.unset = EmbedsMany.prototype.destroyById; EmbedsMany.prototype.at = function (index, cb) { var modelTo = this.definition.modelTo; - var relationName = this.definition.name; + var propertyName = this.definition.keyFrom; var modelInstance = this.modelInstance; - var embeddedList = modelInstance[relationName] || []; + var embeddedList = modelInstance[propertyName] || []; var item = embeddedList[parseInt(index)]; item = (item instanceof modelTo) ? item : null; @@ -1745,9 +1749,9 @@ EmbedsMany.prototype.at = function (index, cb) { }; EmbedsMany.prototype.create = function (targetModelData, cb) { - var pk = this.definition.keyFrom; + var pk = this.definition.keyTo; var modelTo = this.definition.modelTo; - var relationName = this.definition.name; + var propertyName = this.definition.keyFrom; var modelInstance = this.modelInstance; var autoId = this.definition.options.autoId !== false; @@ -1757,7 +1761,7 @@ EmbedsMany.prototype.create = function (targetModelData, cb) { } targetModelData = targetModelData || {}; - var embeddedList = modelInstance[relationName] || []; + var embeddedList = modelInstance[propertyName] || []; var inst = this.build(targetModelData); @@ -1769,20 +1773,20 @@ EmbedsMany.prototype.create = function (targetModelData, cb) { }); } - modelInstance.updateAttribute(relationName, + modelInstance.updateAttribute(propertyName, embeddedList, function(err, modelInst) { cb(err, err ? null : inst); }); }; EmbedsMany.prototype.build = function(targetModelData) { - var pk = this.definition.keyFrom; + var pk = this.definition.keyTo; var modelTo = this.definition.modelTo; - var relationName = this.definition.name; + var propertyName = this.definition.keyFrom; var modelInstance = this.modelInstance; var autoId = this.definition.options.autoId !== false; - var embeddedList = modelInstance[relationName] || []; + var embeddedList = modelInstance[propertyName] || []; targetModelData = targetModelData || {}; @@ -1884,7 +1888,7 @@ EmbedsMany.prototype.remove = function (acInst, cb) { belongsTo.applyScope(modelInstance, filter); - modelInstance[definition.accessor](filter, function(err, items) { + modelInstance[definition.name](filter, function(err, items) { if (err) return cb(err); items.forEach(function(item) { diff --git a/test/loopback-dl.test.js b/test/loopback-dl.test.js index f388e50e..458f91db 100644 --- a/test/loopback-dl.test.js +++ b/test/loopback-dl.test.js @@ -918,7 +918,7 @@ describe('Load models with relations', function () { var ds = new DataSource('memory'); var Post = ds.define('Post', {userId: Number, content: String}); - var User = ds.define('User', {name: String}, {relations: {posts: {type: 'embedsMany', model: 'Post'}}}); + var User = ds.define('User', {name: String}, {relations: {posts: {type: 'embedsMany', model: 'Post' }}}); assert(User.relations['posts']); done(); From 7d847f25dc20d842e859df24f51f2823fe849bce Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Mon, 11 Aug 2014 19:07:48 +0200 Subject: [PATCH 17/34] Fix scopeMethods closure issue --- lib/relation-definition.js | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/relation-definition.js b/lib/relation-definition.js index eeafd328..0074f318 100644 --- a/lib/relation-definition.js +++ b/lib/relation-definition.js @@ -64,12 +64,15 @@ function extendScopeMethods(definition, scopeMethods, ext) { if (typeof ext === 'function') { customMethods = ext.call(definition, scopeMethods, relationClass); } else if (typeof ext === 'object') { + function createFunc(definition, relationMethod) { + return function() { + var relation = new relationClass(definition, this); + return relationMethod.apply(relation, arguments); + }; + }; for (var key in ext) { var relationMethod = ext[key]; - var method = scopeMethods[key] = function () { - var relation = new relationClass(definition, this); - return relationMethod.apply(relation, arguments); - }; + var method = scopeMethods[key] = createFunc(definition, relationMethod); if (relationMethod.shared) { sharedMethod(definition, key, method, relationMethod); } From cd3ad32bb7c41bf9d06dbec222225dc018d9b6c8 Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Wed, 13 Aug 2014 10:10:34 +0200 Subject: [PATCH 18/34] Fix formatting --- lib/relation-definition.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/relation-definition.js b/lib/relation-definition.js index 0074f318..778172eb 100644 --- a/lib/relation-definition.js +++ b/lib/relation-definition.js @@ -65,10 +65,10 @@ function extendScopeMethods(definition, scopeMethods, ext) { customMethods = ext.call(definition, scopeMethods, relationClass); } else if (typeof ext === 'object') { function createFunc(definition, relationMethod) { - return function() { - var relation = new relationClass(definition, this); - return relationMethod.apply(relation, arguments); - }; + return function() { + var relation = new relationClass(definition, this); + return relationMethod.apply(relation, arguments); + }; }; for (var key in ext) { var relationMethod = ext[key]; From 35850f6632684765759721f192026baf5a659907 Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Fri, 15 Aug 2014 11:28:25 +0200 Subject: [PATCH 19/34] Clarified tests, fixed BelongsTo.prototype.create Added clarified test-case based on previous documentation example. Fixed BelongsTo.prototype.create - although the foreignKey was set on the model instance, it was never actually persisted, unless you'd issue a separate call to save the 'parent' model. --- lib/relation-definition.js | 25 +++++++++------- test/relations.test.js | 61 ++++++++++++++++++++++++++++---------- 2 files changed, 60 insertions(+), 26 deletions(-) diff --git a/lib/relation-definition.js b/lib/relation-definition.js index 011b545b..f7af8d2a 100644 --- a/lib/relation-definition.js +++ b/lib/relation-definition.js @@ -155,16 +155,20 @@ RelationDefinition.prototype.applyScope = function(modelInstance, filter) { * @param {Object} modelInstance * @param {Object} target */ -RelationDefinition.prototype.applyProperties = function(modelInstance, target) { +RelationDefinition.prototype.applyProperties = function(modelInstance, obj) { + var source = modelInstance, target = obj; + if (this.options.invertProperties) { + source = obj, target = modelInstance; + } if (typeof this.properties === 'function') { - var data = this.properties.call(this, modelInstance); + var data = this.properties.call(this, source); for(var k in data) { target[k] = data[k]; } } else if (typeof this.properties === 'object') { for(var k in this.properties) { var key = this.properties[k]; - target[key] = modelInstance[k]; + target[key] = source[k]; } } if ((this.type !== 'belongsTo' || this.type === 'hasOne') @@ -1044,14 +1048,17 @@ BelongsTo.prototype.create = function(targetModelData, cb) { cb = targetModelData; targetModelData = {}; } - + this.definition.applyProperties(modelInstance, targetModelData || {}); modelTo.create(targetModelData, function(err, targetModel) { if(!err) { modelInstance[fk] = targetModel[pk]; - self.resetCache(targetModel); - cb && cb(err, targetModel); + modelInstance.save(function(err, inst) { + if (cb && err) return cb && cb(err); + self.resetCache(targetModel); + cb && cb(err, targetModel); + }); } else { cb && cb(err); } @@ -1100,9 +1107,7 @@ BelongsTo.prototype.related = function (refresh, params) { modelInstance[fk] = params[pk]; if (discriminator) modelInstance[discriminator] = params.constructor.modelName; - var data = {}; - this.definition.applyProperties(params, data); - modelInstance.setAttributes(data); + this.definition.applyProperties(modelInstance, params); self.resetCache(params); } else if (typeof params === 'function') { // acts as async getter @@ -1607,7 +1612,7 @@ EmbedsMany.prototype.related = function(receiver, scopeParams, condOrRefresh, cb var params = mergeQuery(actualCond, scopeParams); - if (params.where) { + if (params.where) { // TODO [fabien] Support order/sorting embeddedList = embeddedList ? embeddedList.filter(applyFilter(params)) : embeddedList; } diff --git a/test/relations.test.js b/test/relations.test.js index d1a2003a..da672b1a 100644 --- a/test/relations.test.js +++ b/test/relations.test.js @@ -1109,11 +1109,9 @@ describe('relations', function () { p.person.create({name: 'Fred', age: 36 }, function(err, person) { personCreated = person; p.personId.should.equal(person.id); - p.save(function (err, p) { - person.name.should.equal('Fred'); - person.passportNotes.should.equal('Some notes...'); - done(); - }); + person.name.should.equal('Fred'); + person.passportNotes.should.equal('Some notes...'); + done(); }); }); @@ -1191,7 +1189,6 @@ describe('relations', function () { Article.create(function (e, article) { article.tags.create({name: 'popular'}, function (e, t) { t.should.be.an.instanceOf(Tag); - // console.log(t); ArticleTag.findOne(function (e, at) { should.exist(at); at.tagId.toString().should.equal(t.id.toString()); @@ -1566,19 +1563,15 @@ describe('relations', function () { describe('embedsMany - relations, scope and properties', function () { - var product1, product2, product3; + var category, product1, product2, product3; - before(function (done) { + before(function () { db = getSchema(); Category = db.define('Category', {name: String}); Product = db.define('Product', {name: String}); Link = db.define('Link', {name: String}); - - db.automigrate(function () { - Person.destroyAll(done); - }); }); - + it('can be declared', function (done) { Category.embedsMany(Link, { as: 'items', // rename @@ -1588,6 +1581,7 @@ describe('relations', function () { Link.belongsTo(Product, { foreignKey: 'id', // re-use the actual product id properties: { id: 'id', name: 'name' }, // denormalize, transfer id + options: { invertProperties: true } }); db.automigrate(function() { Product.create({ name: 'Product 0' }, done); // offset ids for tests @@ -1607,7 +1601,7 @@ describe('relations', function () { }); }); - it('should create items on scope', function(done) { + it('should associate items on scope', function(done) { Category.create({ name: 'Category A' }, function(err, cat) { var link = cat.items.build(); link.product(product1); @@ -1718,13 +1712,47 @@ describe('relations', function () { }); }); - it('should remove embedded items by reference id', function(done) { + it('should have removed embedded items by reference id', function(done) { Category.findOne(function(err, cat) { cat.links.should.have.length(1); done(); }); }); + it('should create items on scope', function(done) { + Category.create({ name: 'Category B' }, function(err, cat) { + category = cat; + var link = cat.items.build({ notes: 'Some notes...' }); + link.product.create({ name: 'Product 1' }, function(err, p) { + cat.save(function(err, cat) { // save parent object! + cat.links[0].id.should.eql(p.id); + cat.links[0].name.should.equal('Product 1'); // denormalized + cat.links[0].notes.should.equal('Some notes...'); + cat.items.at(0).should.equal(cat.links[0]); + done(); + }); + }) + }); + }); + + it('should find items on scope', function(done) { + Category.findById(category.id, function(err, cat) { + cat.name.should.equal('Category B'); + cat.links.toObject().should.eql([ + {id: 5, name: 'Product 1', notes: 'Some notes...'} + ]); + cat.items.at(0).should.equal(cat.links[0]); + cat.items(function(err, items) { // alternative access + items.should.be.an.array; + items.should.have.length(1); + items[0].product(function(err, p) { + p.name.should.equal('Product 1'); // actual value + done(); + }); + }); + }); + }); + }); describe('embedsMany - polymorphic relations', function () { @@ -1757,7 +1785,8 @@ describe('relations', function () { }); Link.belongsTo('linked', { polymorphic: true, // needs unique auto-id - properties: { name: 'name' } // denormalized + properties: { name: 'name' }, // denormalized + options: { invertProperties: true } }); db.automigrate(done); }); From c3c2c85ce4e81854e56c398decc5ac6c6aec4b43 Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Fri, 15 Aug 2014 12:55:10 +0200 Subject: [PATCH 20/34] Refactor polymorphic relations, fix inverse #215 See #215 - when creating a related item through a the inverse of a polymorphic HABTM relation, the through-model was not created correctly. By refactoring the specifics into the `polymorphic` property of a RelationDefinition, it's now possible to handle this correctly. --- lib/relation-definition.js | 62 ++++++++++++++++++++++++++------------ test/relations.test.js | 34 +++++++++++++++++++++ 2 files changed, 76 insertions(+), 20 deletions(-) diff --git a/lib/relation-definition.js b/lib/relation-definition.js index f7af8d2a..38aee0ea 100644 --- a/lib/relation-definition.js +++ b/lib/relation-definition.js @@ -98,8 +98,8 @@ function RelationDefinition(definition) { this.keyFrom = definition.keyFrom; this.modelTo = definition.modelTo; this.keyTo = definition.keyTo; - this.discriminator = definition.discriminator; - if (!this.discriminator) { + this.polymorphic = definition.polymorphic; + if (typeof this.polymorphic !== 'object') { assert(this.modelTo, 'Target model is required'); } this.modelThrough = definition.modelThrough; @@ -137,8 +137,13 @@ RelationDefinition.prototype.applyScope = function(modelInstance, filter) { filter = filter || {}; filter.where = filter.where || {}; if ((this.type !== 'belongsTo' || this.type === 'hasOne') - && typeof this.discriminator === 'string') { // polymorphic - filter.where[this.discriminator] = this.modelFrom.modelName; + && typeof this.polymorphic === 'object') { // polymorphic + var discriminator = this.polymorphic.discriminator; + if (this.polymorphic.invert) { + filter.where[discriminator] = this.modelTo.modelName; + } else { + filter.where[discriminator] = this.modelFrom.modelName; + } } if (typeof this.scope === 'function') { var scope = this.scope.call(this, modelInstance, filter); @@ -172,8 +177,13 @@ RelationDefinition.prototype.applyProperties = function(modelInstance, obj) { } } if ((this.type !== 'belongsTo' || this.type === 'hasOne') - && typeof this.discriminator === 'string') { // polymorphic - target[this.discriminator] = this.modelFrom.modelName; + && typeof this.polymorphic === 'object') { // polymorphic + var discriminator = this.polymorphic.discriminator; + if (this.polymorphic.invert) { + target[discriminator] = this.modelTo.modelName; + } else { + target[discriminator] = this.modelFrom.modelName; + } } }; @@ -461,10 +471,11 @@ RelationDefinition.hasMany = function hasMany(modelFrom, modelTo, params) { var fk = params.foreignKey || i8n.camelize(thisClassName + '_id', true); var idName = modelFrom.dataSource.idName(modelFrom.modelName) || 'id'; - var discriminator; + var discriminator, polymorphic; if (params.polymorphic) { - var polymorphic = polymorphicParams(params.polymorphic); + polymorphic = polymorphicParams(params.polymorphic); + polymorphic.invert = !!params.invert; discriminator = polymorphic.discriminator; if (!params.invert) { fk = polymorphic.foreignKey; @@ -480,12 +491,12 @@ RelationDefinition.hasMany = function hasMany(modelFrom, modelTo, params) { modelFrom: modelFrom, keyFrom: idName, keyTo: fk, - discriminator: discriminator, modelTo: modelTo, multiple: true, properties: params.properties, scope: params.scope, - options: params.options + options: params.options, + polymorphic: polymorphic }); definition.modelThrough = params.through; @@ -700,9 +711,13 @@ var throughKeys = function(definition) { var modelThrough = definition.modelThrough; var pk2 = definition.modelTo.definition.idName(); - if (definition.discriminator) { // polymorphic + if (typeof definition.polymorphic === 'object') { // polymorphic var fk1 = definition.keyTo; - var fk2 = definition.keyThrough; + if (definition.polymorphic.invert) { + var fk2 = definition.polymorphic.foreignKey; + } else { + var fk2 = definition.keyThrough; + } } else { var fk1 = findBelongsTo(modelThrough, definition.modelFrom, definition.keyFrom); @@ -1004,11 +1019,11 @@ RelationDefinition.belongsTo = function (modelFrom, modelTo, params) { modelFrom: modelFrom, keyFrom: fk, keyTo: idName, - discriminator: discriminator, modelTo: modelTo, properties: params.properties, scope: params.scope, - options: params.options + options: params.options, + polymorphic: polymorphic }); // Define a property for the scope so that we have 'this' for the scoped methods @@ -1086,10 +1101,10 @@ BelongsTo.prototype.related = function (refresh, params) { var self = this; var modelFrom = this.definition.modelFrom; var modelTo = this.definition.modelTo; - var discriminator = this.definition.discriminator; var pk = this.definition.keyTo; var fk = this.definition.keyFrom; var modelInstance = this.modelInstance; + var discriminator; if (arguments.length === 1) { params = refresh; @@ -1098,6 +1113,10 @@ BelongsTo.prototype.related = function (refresh, params) { throw new Error('Method can\'t be called with more than two arguments'); } + if (typeof this.definition.polymorphic === 'object') { + discriminator = this.definition.polymorphic.discriminator; + } + var cachedValue; if (!refresh) { cachedValue = self.getCache(); @@ -1105,7 +1124,10 @@ BelongsTo.prototype.related = function (refresh, params) { if (params instanceof ModelBaseClass) { // acts as setter modelTo = params.constructor; modelInstance[fk] = params[pk]; - if (discriminator) modelInstance[discriminator] = params.constructor.modelName; + + if (discriminator) { + modelInstance[discriminator] = params.constructor.modelName; + } this.definition.applyProperties(modelInstance, params); @@ -1245,10 +1267,10 @@ RelationDefinition.hasOne = function (modelFrom, modelTo, params) { var relationName = params.as || i8n.camelize(modelTo.modelName, true); var fk = params.foreignKey || i8n.camelize(modelFrom.modelName + '_id', true); - var discriminator; + var discriminator, polymorphic; if (params.polymorphic) { - var polymorphic = polymorphicParams(params.polymorphic); + polymorphic = polymorphicParams(params.polymorphic); fk = polymorphic.foreignKey; discriminator = polymorphic.discriminator; if (!params.through) { @@ -1262,10 +1284,10 @@ RelationDefinition.hasOne = function (modelFrom, modelTo, params) { modelFrom: modelFrom, keyFrom: pk, keyTo: fk, - discriminator: discriminator, modelTo: modelTo, properties: params.properties, - options: params.options + options: params.options, + polymorphic: polymorphic }); modelFrom.dataSource.defineForeignKey(modelTo.modelName, fk, modelFrom.modelName); diff --git a/test/relations.test.js b/test/relations.test.js index da672b1a..5887adb0 100644 --- a/test/relations.test.js +++ b/test/relations.test.js @@ -1030,6 +1030,40 @@ describe('relations', function () { }); }); }); + + it('should create polymorphic item through relation scope', function (done) { + Picture.findById(anotherPicture.id, function(err, p) { + p.authors.create({ name: 'Author 3' }, function(err, a) { + should.not.exist(err); + author = a; + author.name.should.equal('Author 3'); + done(); + }); + }); + }); + + it('should create polymorphic through model - new author', function (done) { + PictureLink.findOne({ where: { + pictureId: anotherPicture.id, imageableId: author.id, imageableType: 'Author' + } }, function(err, link) { + should.not.exist(err); + link.pictureId.should.eql(anotherPicture.id); + link.imageableId.should.eql(author.id); + link.imageableType.should.equal('Author'); + done(); + }); + }); + + it('should find polymorphic items - new author', function (done) { + Author.findById(author.id, function(err, author) { + author.pictures(function(err, pics) { + pics.should.have.length(1); + pics[0].id.should.eql(anotherPicture.id); + pics[0].name.should.equal('Example'); + done(); + }); + }); + }); }); From 21801058c9aa75f67a86924416a59bc377b835c1 Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Fri, 15 Aug 2014 15:12:02 +0200 Subject: [PATCH 21/34] Refactor embedsMany - auto-save parent With this change, saving an embedded model now correctly updates the parent model. Before, a separate `save()` call on the parent was required, contrary to other relation types. --- lib/relation-definition.js | 62 ++++++++++++++++++++++++++++++-------- test/relations.test.js | 34 ++++++++++++++++----- 2 files changed, 75 insertions(+), 21 deletions(-) diff --git a/lib/relation-definition.js b/lib/relation-definition.js index 38aee0ea..2e5f3766 100644 --- a/lib/relation-definition.js +++ b/lib/relation-definition.js @@ -1607,11 +1607,48 @@ RelationDefinition.embedsMany = function embedsMany(modelFrom, modelTo, params) scopeDefinition.related = scopeMethods.related; }; +EmbedsMany.prototype.prepareEmbeddedInstance = function(inst) { + if (inst && inst.triggerParent !== 'function') { + var relationName = this.definition.name; + var modelInstance = this.modelInstance; + inst.triggerParent = function(actionName, callback) { + if (actionName === 'save' || actionName === 'destroy') { + var embeddedList = modelInstance[relationName] || []; + modelInstance.updateAttribute(relationName, + embeddedList, function(err, modelInst) { + callback(err, err ? null : modelInst); + }); + } else { + process.nextTick(callback); + } + }; + var originalTrigger = inst.trigger; + inst.trigger = function(actionName, work, data, callback) { + if (typeof work === 'function') { + var originalWork = work; + work = function(next) { + originalWork.call(this, function(done) { + inst.triggerParent(actionName, function(err, inst) { + next(done); // TODO [fabien] - error handling? + }); + }); + }; + } + originalTrigger.call(this, actionName, work, data, callback); + }; + } +}; + +EmbedsMany.prototype.embeddedList = function(modelInstance) { + modelInstance = modelInstance || this.modelInstance; + var embeddedList = modelInstance[this.definition.name] || []; + embeddedList.forEach(this.prepareEmbeddedInstance.bind(this)); + return embeddedList; +}; + EmbedsMany.prototype.related = function(receiver, scopeParams, condOrRefresh, cb) { var modelTo = this.definition.modelTo; - var relationName = this.definition.name; var modelInstance = this.modelInstance; - var self = receiver; var actualCond = {}; var actualRefresh = false; @@ -1628,9 +1665,9 @@ EmbedsMany.prototype.related = function(receiver, scopeParams, condOrRefresh, cb throw new Error('Method can be only called with one or two arguments'); } - var embeddedList = self[relationName] || []; + var embeddedList = this.embeddedList(receiver); - this.definition.applyScope(modelInstance, actualCond); + this.definition.applyScope(receiver, actualCond); var params = mergeQuery(actualCond, scopeParams); @@ -1652,10 +1689,9 @@ EmbedsMany.prototype.related = function(receiver, scopeParams, condOrRefresh, cb EmbedsMany.prototype.findById = function (fkId, cb) { var pk = this.definition.keyFrom; var modelTo = this.definition.modelTo; - var relationName = this.definition.name; var modelInstance = this.modelInstance; - var embeddedList = modelInstance[relationName] || []; + var embeddedList = this.embeddedList(); var find = function(id) { for (var i = 0; i < embeddedList.length; i++) { @@ -1695,7 +1731,7 @@ EmbedsMany.prototype.updateById = function (fkId, data, cb) { var relationName = this.definition.name; var modelInstance = this.modelInstance; - var embeddedList = modelInstance[relationName] || []; + var embeddedList = this.embeddedList(); var inst = this.findById(fkId); @@ -1729,7 +1765,7 @@ EmbedsMany.prototype.destroyById = function (fkId, cb) { var relationName = this.definition.name; var modelInstance = this.modelInstance; - var embeddedList = modelInstance[relationName] || []; + var embeddedList = this.embeddedList(); var inst = (fkId instanceof modelTo) ? fkId : this.findById(fkId); @@ -1754,10 +1790,9 @@ EmbedsMany.prototype.unset = EmbedsMany.prototype.destroyById; EmbedsMany.prototype.at = function (index, cb) { var modelTo = this.definition.modelTo; - var relationName = this.definition.name; var modelInstance = this.modelInstance; - var embeddedList = modelInstance[relationName] || []; + var embeddedList = this.embeddedList(); var item = embeddedList[parseInt(index)]; item = (item instanceof modelTo) ? item : null; @@ -1784,7 +1819,7 @@ EmbedsMany.prototype.create = function (targetModelData, cb) { } targetModelData = targetModelData || {}; - var embeddedList = modelInstance[relationName] || []; + var embeddedList = this.embeddedList(); var inst = this.build(targetModelData); @@ -1805,11 +1840,10 @@ EmbedsMany.prototype.create = function (targetModelData, cb) { EmbedsMany.prototype.build = function(targetModelData) { var pk = this.definition.keyFrom; var modelTo = this.definition.modelTo; - var relationName = this.definition.name; var modelInstance = this.modelInstance; var autoId = this.definition.options.autoId !== false; - var embeddedList = modelInstance[relationName] || []; + var embeddedList = this.embeddedList(); targetModelData = targetModelData || {}; @@ -1834,6 +1868,8 @@ EmbedsMany.prototype.build = function(targetModelData) { embeddedList.push(inst); } + this.prepareEmbeddedInstance(inst); + return inst; }; diff --git a/test/relations.test.js b/test/relations.test.js index 5887adb0..7f239f33 100644 --- a/test/relations.test.js +++ b/test/relations.test.js @@ -1758,14 +1758,12 @@ describe('relations', function () { category = cat; var link = cat.items.build({ notes: 'Some notes...' }); link.product.create({ name: 'Product 1' }, function(err, p) { - cat.save(function(err, cat) { // save parent object! - cat.links[0].id.should.eql(p.id); - cat.links[0].name.should.equal('Product 1'); // denormalized - cat.links[0].notes.should.equal('Some notes...'); - cat.items.at(0).should.equal(cat.links[0]); - done(); - }); - }) + cat.links[0].id.should.eql(p.id); + cat.links[0].name.should.equal('Product 1'); // denormalized + cat.links[0].notes.should.equal('Some notes...'); + cat.items.at(0).should.equal(cat.links[0]); + done(); + }); }); }); @@ -1787,6 +1785,26 @@ describe('relations', function () { }); }); + it('should update items on scope - and save parent', function(done) { + Category.findById(category.id, function(err, cat) { + var link = cat.items.at(0); + link.updateAttributes({notes: 'Updated notes...'}, function(err, link) { + link.notes.should.equal('Updated notes...'); + done(); + }); + }); + }); + + it('should find items on scope - verify update', function(done) { + Category.findById(category.id, function(err, cat) { + cat.name.should.equal('Category B'); + cat.links.toObject().should.eql([ + {id: 5, name: 'Product 1', notes: 'Updated notes...'} + ]); + done(); + }); + }); + }); describe('embedsMany - polymorphic relations', function () { From c2f9ee381cc7b470e2cf230a4e00523f11d638b3 Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Fri, 15 Aug 2014 15:24:00 +0200 Subject: [PATCH 22/34] Implement embedded.destroy() integration --- lib/relation-definition.js | 7 ++++++- test/relations.test.js | 17 +++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/lib/relation-definition.js b/lib/relation-definition.js index 2e5f3766..65843c29 100644 --- a/lib/relation-definition.js +++ b/lib/relation-definition.js @@ -1609,11 +1609,16 @@ RelationDefinition.embedsMany = function embedsMany(modelFrom, modelTo, params) EmbedsMany.prototype.prepareEmbeddedInstance = function(inst) { if (inst && inst.triggerParent !== 'function') { + var self = this; var relationName = this.definition.name; var modelInstance = this.modelInstance; inst.triggerParent = function(actionName, callback) { if (actionName === 'save' || actionName === 'destroy') { - var embeddedList = modelInstance[relationName] || []; + var embeddedList = self.embeddedList(); + if (actionName === 'destroy') { + var index = embeddedList.indexOf(inst); + if (index > -1) embeddedList.splice(index, 1); + } modelInstance.updateAttribute(relationName, embeddedList, function(err, modelInst) { callback(err, err ? null : modelInst); diff --git a/test/relations.test.js b/test/relations.test.js index 7f239f33..53f04229 100644 --- a/test/relations.test.js +++ b/test/relations.test.js @@ -1805,6 +1805,23 @@ describe('relations', function () { }); }); + it('should remove items from scope - and save parent', function(done) { + Category.findById(category.id, function(err, cat) { + cat.items.at(0).destroy(function(err, link) { + cat.links.should.eql([]); + done(); + }); + }); + }); + + it('should find items on scope - verify destroy', function(done) { + Category.findById(category.id, function(err, cat) { + cat.name.should.equal('Category B'); + cat.links.should.eql([]); + done(); + }); + }); + }); describe('embedsMany - polymorphic relations', function () { From 7cd880712b9242cb002e7357e22b48b19a94ec75 Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Fri, 15 Aug 2014 18:01:40 +0200 Subject: [PATCH 23/34] Handle toObject in updateAttributes Since one can call updateAttributes with any kind of properties (as opposed to save, which uses toObject internally), any objects that correspond to toObject should be handled as such. This is particularly the case with List objects, as used by embedsMany. --- lib/dao.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/dao.js b/lib/dao.js index 4ed71b3e..00fbf18c 100644 --- a/lib/dao.js +++ b/lib/dao.js @@ -1175,6 +1175,10 @@ DataAccessObject.prototype.updateAttributes = function updateAttributes(data, cb // Convert the properties by type inst[key] = data[key]; typedData[key] = inst[key]; + if (typeof typedData[key] === 'object' + && typeof typedData[key].toObject === 'function') { + typedData[key] = typedData[key].toObject(); + } } inst._adapter().updateAttributes(model, getIdValue(inst.constructor, inst), From 78e2c9c9d47f987ef56cdb4f043461a406a2e324 Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Fri, 15 Aug 2014 11:28:25 +0200 Subject: [PATCH 24/34] Clarified tests, fixed BelongsTo.prototype.create Added clarified test-case based on previous documentation example. Fixed BelongsTo.prototype.create - although the foreignKey was set on the model instance, it was never actually persisted, unless you'd issue a separate call to save the 'parent' model. --- lib/relation-definition.js | 25 +++++++++------- test/relations.test.js | 61 ++++++++++++++++++++++++++++---------- 2 files changed, 60 insertions(+), 26 deletions(-) diff --git a/lib/relation-definition.js b/lib/relation-definition.js index 93d1b6ca..edc833d2 100644 --- a/lib/relation-definition.js +++ b/lib/relation-definition.js @@ -181,16 +181,20 @@ RelationDefinition.prototype.applyScope = function(modelInstance, filter) { * @param {Object} modelInstance * @param {Object} target */ -RelationDefinition.prototype.applyProperties = function(modelInstance, target) { +RelationDefinition.prototype.applyProperties = function(modelInstance, obj) { + var source = modelInstance, target = obj; + if (this.options.invertProperties) { + source = obj, target = modelInstance; + } if (typeof this.properties === 'function') { - var data = this.properties.call(this, modelInstance); + var data = this.properties.call(this, source); for(var k in data) { target[k] = data[k]; } } else if (typeof this.properties === 'object') { for(var k in this.properties) { var key = this.properties[k]; - target[key] = modelInstance[k]; + target[key] = source[k]; } } if ((this.type !== 'belongsTo' || this.type === 'hasOne') @@ -1082,14 +1086,17 @@ BelongsTo.prototype.create = function(targetModelData, cb) { cb = targetModelData; targetModelData = {}; } - + this.definition.applyProperties(modelInstance, targetModelData || {}); modelTo.create(targetModelData, function(err, targetModel) { if(!err) { modelInstance[fk] = targetModel[pk]; - self.resetCache(targetModel); - cb && cb(err, targetModel); + modelInstance.save(function(err, inst) { + if (cb && err) return cb && cb(err); + self.resetCache(targetModel); + cb && cb(err, targetModel); + }); } else { cb && cb(err); } @@ -1138,9 +1145,7 @@ BelongsTo.prototype.related = function (refresh, params) { modelInstance[fk] = params[pk]; if (discriminator) modelInstance[discriminator] = params.constructor.modelName; - var data = {}; - this.definition.applyProperties(params, data); - modelInstance.setAttributes(data); + this.definition.applyProperties(modelInstance, params); self.resetCache(params); } else if (typeof params === 'function') { // acts as async getter @@ -1652,7 +1657,7 @@ EmbedsMany.prototype.related = function(receiver, scopeParams, condOrRefresh, cb var params = mergeQuery(actualCond, scopeParams); - if (params.where) { + if (params.where) { // TODO [fabien] Support order/sorting embeddedList = embeddedList ? embeddedList.filter(applyFilter(params)) : embeddedList; } diff --git a/test/relations.test.js b/test/relations.test.js index ce0e62f3..458382b4 100644 --- a/test/relations.test.js +++ b/test/relations.test.js @@ -1109,11 +1109,9 @@ describe('relations', function () { p.person.create({name: 'Fred', age: 36 }, function(err, person) { personCreated = person; p.personId.should.equal(person.id); - p.save(function (err, p) { - person.name.should.equal('Fred'); - person.passportNotes.should.equal('Some notes...'); - done(); - }); + person.name.should.equal('Fred'); + person.passportNotes.should.equal('Some notes...'); + done(); }); }); @@ -1191,7 +1189,6 @@ describe('relations', function () { Article.create(function (e, article) { article.tags.create({name: 'popular'}, function (e, t) { t.should.be.an.instanceOf(Tag); - // console.log(t); ArticleTag.findOne(function (e, at) { should.exist(at); at.tagId.toString().should.equal(t.id.toString()); @@ -1566,19 +1563,15 @@ describe('relations', function () { describe('embedsMany - relations, scope and properties', function () { - var product1, product2, product3; + var category, product1, product2, product3; - before(function (done) { + before(function () { db = getSchema(); Category = db.define('Category', {name: String}); Product = db.define('Product', {name: String}); Link = db.define('Link', {name: String}); - - db.automigrate(function () { - Person.destroyAll(done); - }); }); - + it('can be declared', function (done) { Category.embedsMany(Link, { as: 'items', // rename @@ -1588,6 +1581,7 @@ describe('relations', function () { Link.belongsTo(Product, { foreignKey: 'id', // re-use the actual product id properties: { id: 'id', name: 'name' }, // denormalize, transfer id + options: { invertProperties: true } }); db.automigrate(function() { Product.create({ name: 'Product 0' }, done); // offset ids for tests @@ -1607,7 +1601,7 @@ describe('relations', function () { }); }); - it('should create items on scope', function(done) { + it('should associate items on scope', function(done) { Category.create({ name: 'Category A' }, function(err, cat) { var link = cat.items.build(); link.product(product1); @@ -1718,13 +1712,47 @@ describe('relations', function () { }); }); - it('should remove embedded items by reference id', function(done) { + it('should have removed embedded items by reference id', function(done) { Category.findOne(function(err, cat) { cat.links.should.have.length(1); done(); }); }); + it('should create items on scope', function(done) { + Category.create({ name: 'Category B' }, function(err, cat) { + category = cat; + var link = cat.items.build({ notes: 'Some notes...' }); + link.product.create({ name: 'Product 1' }, function(err, p) { + cat.save(function(err, cat) { // save parent object! + cat.links[0].id.should.eql(p.id); + cat.links[0].name.should.equal('Product 1'); // denormalized + cat.links[0].notes.should.equal('Some notes...'); + cat.items.at(0).should.equal(cat.links[0]); + done(); + }); + }) + }); + }); + + it('should find items on scope', function(done) { + Category.findById(category.id, function(err, cat) { + cat.name.should.equal('Category B'); + cat.links.toObject().should.eql([ + {id: 5, name: 'Product 1', notes: 'Some notes...'} + ]); + cat.items.at(0).should.equal(cat.links[0]); + cat.items(function(err, items) { // alternative access + items.should.be.an.array; + items.should.have.length(1); + items[0].product(function(err, p) { + p.name.should.equal('Product 1'); // actual value + done(); + }); + }); + }); + }); + }); describe('embedsMany - polymorphic relations', function () { @@ -1757,7 +1785,8 @@ describe('relations', function () { }); Link.belongsTo('linked', { polymorphic: true, // needs unique auto-id - properties: { name: 'name' } // denormalized + properties: { name: 'name' }, // denormalized + options: { invertProperties: true } }); db.automigrate(done); }); From 8193f402bbdab896b5d464c070a8df6f3e3fe33f Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Fri, 15 Aug 2014 12:55:10 +0200 Subject: [PATCH 25/34] Refactor polymorphic relations, fix inverse #215 See #215 - when creating a related item through a the inverse of a polymorphic HABTM relation, the through-model was not created correctly. By refactoring the specifics into the `polymorphic` property of a RelationDefinition, it's now possible to handle this correctly. --- lib/relation-definition.js | 62 ++++++++++++++++++++++++++------------ test/relations.test.js | 34 +++++++++++++++++++++ 2 files changed, 76 insertions(+), 20 deletions(-) diff --git a/lib/relation-definition.js b/lib/relation-definition.js index edc833d2..f824a8a5 100644 --- a/lib/relation-definition.js +++ b/lib/relation-definition.js @@ -101,8 +101,8 @@ function RelationDefinition(definition) { this.keyFrom = definition.keyFrom; this.modelTo = definition.modelTo; this.keyTo = definition.keyTo; - this.discriminator = definition.discriminator; - if (!this.discriminator) { + this.polymorphic = definition.polymorphic; + if (typeof this.polymorphic !== 'object') { assert(this.modelTo, 'Target model is required'); } this.modelThrough = definition.modelThrough; @@ -163,8 +163,13 @@ RelationDefinition.prototype.applyScope = function(modelInstance, filter) { filter = filter || {}; filter.where = filter.where || {}; if ((this.type !== 'belongsTo' || this.type === 'hasOne') - && typeof this.discriminator === 'string') { // polymorphic - filter.where[this.discriminator] = this.modelFrom.modelName; + && typeof this.polymorphic === 'object') { // polymorphic + var discriminator = this.polymorphic.discriminator; + if (this.polymorphic.invert) { + filter.where[discriminator] = this.modelTo.modelName; + } else { + filter.where[discriminator] = this.modelFrom.modelName; + } } if (typeof this.scope === 'function') { var scope = this.scope.call(this, modelInstance, filter); @@ -198,8 +203,13 @@ RelationDefinition.prototype.applyProperties = function(modelInstance, obj) { } } if ((this.type !== 'belongsTo' || this.type === 'hasOne') - && typeof this.discriminator === 'string') { // polymorphic - target[this.discriminator] = this.modelFrom.modelName; + && typeof this.polymorphic === 'object') { // polymorphic + var discriminator = this.polymorphic.discriminator; + if (this.polymorphic.invert) { + target[discriminator] = this.modelTo.modelName; + } else { + target[discriminator] = this.modelFrom.modelName; + } } }; @@ -496,10 +506,11 @@ RelationDefinition.hasMany = function hasMany(modelFrom, modelTo, params) { var fk = params.foreignKey || i8n.camelize(thisClassName + '_id', true); var idName = modelFrom.dataSource.idName(modelFrom.modelName) || 'id'; - var discriminator; + var discriminator, polymorphic; if (params.polymorphic) { - var polymorphic = polymorphicParams(params.polymorphic); + polymorphic = polymorphicParams(params.polymorphic); + polymorphic.invert = !!params.invert; discriminator = polymorphic.discriminator; if (!params.invert) { fk = polymorphic.foreignKey; @@ -515,12 +526,12 @@ RelationDefinition.hasMany = function hasMany(modelFrom, modelTo, params) { modelFrom: modelFrom, keyFrom: idName, keyTo: fk, - discriminator: discriminator, modelTo: modelTo, multiple: true, properties: params.properties, scope: params.scope, - options: params.options + options: params.options, + polymorphic: polymorphic }); definition.modelThrough = params.through; @@ -736,9 +747,13 @@ var throughKeys = function(definition) { var modelThrough = definition.modelThrough; var pk2 = definition.modelTo.definition.idName(); - if (definition.discriminator) { // polymorphic + if (typeof definition.polymorphic === 'object') { // polymorphic var fk1 = definition.keyTo; - var fk2 = definition.keyThrough; + if (definition.polymorphic.invert) { + var fk2 = definition.polymorphic.foreignKey; + } else { + var fk2 = definition.keyThrough; + } } else { var fk1 = findBelongsTo(modelThrough, definition.modelFrom, definition.keyFrom); @@ -1040,11 +1055,11 @@ RelationDefinition.belongsTo = function (modelFrom, modelTo, params) { modelFrom: modelFrom, keyFrom: fk, keyTo: idName, - discriminator: discriminator, modelTo: modelTo, properties: params.properties, scope: params.scope, - options: params.options + options: params.options, + polymorphic: polymorphic }); // Define a property for the scope so that we have 'this' for the scoped methods @@ -1124,10 +1139,10 @@ BelongsTo.prototype.related = function (refresh, params) { var self = this; var modelFrom = this.definition.modelFrom; var modelTo = this.definition.modelTo; - var discriminator = this.definition.discriminator; var pk = this.definition.keyTo; var fk = this.definition.keyFrom; var modelInstance = this.modelInstance; + var discriminator; if (arguments.length === 1) { params = refresh; @@ -1136,6 +1151,10 @@ BelongsTo.prototype.related = function (refresh, params) { throw new Error('Method can\'t be called with more than two arguments'); } + if (typeof this.definition.polymorphic === 'object') { + discriminator = this.definition.polymorphic.discriminator; + } + var cachedValue; if (!refresh) { cachedValue = self.getCache(); @@ -1143,7 +1162,10 @@ BelongsTo.prototype.related = function (refresh, params) { if (params instanceof ModelBaseClass) { // acts as setter modelTo = params.constructor; modelInstance[fk] = params[pk]; - if (discriminator) modelInstance[discriminator] = params.constructor.modelName; + + if (discriminator) { + modelInstance[discriminator] = params.constructor.modelName; + } this.definition.applyProperties(modelInstance, params); @@ -1282,10 +1304,10 @@ RelationDefinition.hasOne = function (modelFrom, modelTo, params) { var relationName = params.as || i8n.camelize(modelTo.modelName, true); var fk = params.foreignKey || i8n.camelize(modelFrom.modelName + '_id', true); - var discriminator; + var discriminator, polymorphic; if (params.polymorphic) { - var polymorphic = polymorphicParams(params.polymorphic); + polymorphic = polymorphicParams(params.polymorphic); fk = polymorphic.foreignKey; discriminator = polymorphic.discriminator; if (!params.through) { @@ -1299,10 +1321,10 @@ RelationDefinition.hasOne = function (modelFrom, modelTo, params) { modelFrom: modelFrom, keyFrom: pk, keyTo: fk, - discriminator: discriminator, modelTo: modelTo, properties: params.properties, - options: params.options + options: params.options, + polymorphic: polymorphic }); modelFrom.dataSource.defineForeignKey(modelTo.modelName, fk, modelFrom.modelName); diff --git a/test/relations.test.js b/test/relations.test.js index 458382b4..7db9957f 100644 --- a/test/relations.test.js +++ b/test/relations.test.js @@ -1030,6 +1030,40 @@ describe('relations', function () { }); }); }); + + it('should create polymorphic item through relation scope', function (done) { + Picture.findById(anotherPicture.id, function(err, p) { + p.authors.create({ name: 'Author 3' }, function(err, a) { + should.not.exist(err); + author = a; + author.name.should.equal('Author 3'); + done(); + }); + }); + }); + + it('should create polymorphic through model - new author', function (done) { + PictureLink.findOne({ where: { + pictureId: anotherPicture.id, imageableId: author.id, imageableType: 'Author' + } }, function(err, link) { + should.not.exist(err); + link.pictureId.should.eql(anotherPicture.id); + link.imageableId.should.eql(author.id); + link.imageableType.should.equal('Author'); + done(); + }); + }); + + it('should find polymorphic items - new author', function (done) { + Author.findById(author.id, function(err, author) { + author.pictures(function(err, pics) { + pics.should.have.length(1); + pics[0].id.should.eql(anotherPicture.id); + pics[0].name.should.equal('Example'); + done(); + }); + }); + }); }); From 96a276a12b4a75a85b1092ccbd4ad0b31c5fd056 Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Fri, 15 Aug 2014 15:12:02 +0200 Subject: [PATCH 26/34] Refactor embedsMany - auto-save parent With this change, saving an embedded model now correctly updates the parent model. Before, a separate `save()` call on the parent was required, contrary to other relation types. --- lib/relation-definition.js | 67 ++++++++++++++++++++++++++++++-------- test/relations.test.js | 34 ++++++++++++++----- 2 files changed, 80 insertions(+), 21 deletions(-) diff --git a/lib/relation-definition.js b/lib/relation-definition.js index f824a8a5..e11b28bc 100644 --- a/lib/relation-definition.js +++ b/lib/relation-definition.js @@ -1652,11 +1652,53 @@ RelationDefinition.embedsMany = function embedsMany(modelFrom, modelTo, params) return definition; }; +EmbedsMany.prototype.prepareEmbeddedInstance = function(inst) { + if (inst && inst.triggerParent !== 'function') { + var self = this; + var propertyName = this.definition.keyFrom; + var modelInstance = this.modelInstance; + inst.triggerParent = function(actionName, callback) { + if (actionName === 'save' || actionName === 'destroy') { + var embeddedList = self.embeddedList(); + if (actionName === 'destroy') { + var index = embeddedList.indexOf(inst); + if (index > -1) embeddedList.splice(index, 1); + } + modelInstance.updateAttribute(propertyName, + embeddedList, function(err, modelInst) { + callback(err, err ? null : modelInst); + }); + } else { + process.nextTick(callback); + } + }; + var originalTrigger = inst.trigger; + inst.trigger = function(actionName, work, data, callback) { + if (typeof work === 'function') { + var originalWork = work; + work = function(next) { + originalWork.call(this, function(done) { + inst.triggerParent(actionName, function(err, inst) { + next(done); // TODO [fabien] - error handling? + }); + }); + }; + } + originalTrigger.call(this, actionName, work, data, callback); + }; + } +}; + +EmbedsMany.prototype.embeddedList = function(modelInstance) { + modelInstance = modelInstance || this.modelInstance; + var embeddedList = modelInstance[this.definition.keyFrom] || []; + embeddedList.forEach(this.prepareEmbeddedInstance.bind(this)); + return embeddedList; +}; + EmbedsMany.prototype.related = function(receiver, scopeParams, condOrRefresh, cb) { var modelTo = this.definition.modelTo; - var propertyName = this.definition.keyFrom; var modelInstance = this.modelInstance; - var self = receiver; var actualCond = {}; var actualRefresh = false; @@ -1673,9 +1715,9 @@ EmbedsMany.prototype.related = function(receiver, scopeParams, condOrRefresh, cb throw new Error('Method can be only called with one or two arguments'); } - var embeddedList = self[propertyName] || []; + var embeddedList = this.embeddedList(receiver); - this.definition.applyScope(modelInstance, actualCond); + this.definition.applyScope(receiver, actualCond); var params = mergeQuery(actualCond, scopeParams); @@ -1697,10 +1739,9 @@ EmbedsMany.prototype.related = function(receiver, scopeParams, condOrRefresh, cb EmbedsMany.prototype.findById = function (fkId, cb) { var pk = this.definition.keyTo; var modelTo = this.definition.modelTo; - var propertyName = this.definition.keyFrom; var modelInstance = this.modelInstance; - var embeddedList = modelInstance[propertyName] || []; + var embeddedList = this.embeddedList(); var find = function(id) { for (var i = 0; i < embeddedList.length; i++) { @@ -1740,7 +1781,7 @@ EmbedsMany.prototype.updateById = function (fkId, data, cb) { var propertyName = this.definition.keyFrom; var modelInstance = this.modelInstance; - var embeddedList = modelInstance[propertyName] || []; + var embeddedList = this.embeddedList(); var inst = this.findById(fkId); @@ -1774,7 +1815,7 @@ EmbedsMany.prototype.destroyById = function (fkId, cb) { var propertyName = this.definition.keyFrom; var modelInstance = this.modelInstance; - var embeddedList = modelInstance[propertyName] || []; + var embeddedList = this.embeddedList(); var inst = (fkId instanceof modelTo) ? fkId : this.findById(fkId); @@ -1799,10 +1840,9 @@ EmbedsMany.prototype.unset = EmbedsMany.prototype.destroyById; EmbedsMany.prototype.at = function (index, cb) { var modelTo = this.definition.modelTo; - var propertyName = this.definition.keyFrom; var modelInstance = this.modelInstance; - var embeddedList = modelInstance[propertyName] || []; + var embeddedList = this.embeddedList(); var item = embeddedList[parseInt(index)]; item = (item instanceof modelTo) ? item : null; @@ -1829,7 +1869,7 @@ EmbedsMany.prototype.create = function (targetModelData, cb) { } targetModelData = targetModelData || {}; - var embeddedList = modelInstance[propertyName] || []; + var embeddedList = this.embeddedList(); var inst = this.build(targetModelData); @@ -1850,11 +1890,10 @@ EmbedsMany.prototype.create = function (targetModelData, cb) { EmbedsMany.prototype.build = function(targetModelData) { var pk = this.definition.keyTo; var modelTo = this.definition.modelTo; - var propertyName = this.definition.keyFrom; var modelInstance = this.modelInstance; var autoId = this.definition.options.autoId !== false; - var embeddedList = modelInstance[propertyName] || []; + var embeddedList = this.embeddedList(); targetModelData = targetModelData || {}; @@ -1879,6 +1918,8 @@ EmbedsMany.prototype.build = function(targetModelData) { embeddedList.push(inst); } + this.prepareEmbeddedInstance(inst); + return inst; }; diff --git a/test/relations.test.js b/test/relations.test.js index 7db9957f..55a8e4cf 100644 --- a/test/relations.test.js +++ b/test/relations.test.js @@ -1758,14 +1758,12 @@ describe('relations', function () { category = cat; var link = cat.items.build({ notes: 'Some notes...' }); link.product.create({ name: 'Product 1' }, function(err, p) { - cat.save(function(err, cat) { // save parent object! - cat.links[0].id.should.eql(p.id); - cat.links[0].name.should.equal('Product 1'); // denormalized - cat.links[0].notes.should.equal('Some notes...'); - cat.items.at(0).should.equal(cat.links[0]); - done(); - }); - }) + cat.links[0].id.should.eql(p.id); + cat.links[0].name.should.equal('Product 1'); // denormalized + cat.links[0].notes.should.equal('Some notes...'); + cat.items.at(0).should.equal(cat.links[0]); + done(); + }); }); }); @@ -1787,6 +1785,26 @@ describe('relations', function () { }); }); + it('should update items on scope - and save parent', function(done) { + Category.findById(category.id, function(err, cat) { + var link = cat.items.at(0); + link.updateAttributes({notes: 'Updated notes...'}, function(err, link) { + link.notes.should.equal('Updated notes...'); + done(); + }); + }); + }); + + it('should find items on scope - verify update', function(done) { + Category.findById(category.id, function(err, cat) { + cat.name.should.equal('Category B'); + cat.links.toObject().should.eql([ + {id: 5, name: 'Product 1', notes: 'Updated notes...'} + ]); + done(); + }); + }); + }); describe('embedsMany - polymorphic relations', function () { From 94310549cf08c8b2458160ce8df852b3ad2eb447 Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Fri, 15 Aug 2014 15:24:00 +0200 Subject: [PATCH 27/34] Implement embedded.destroy() integration --- test/relations.test.js | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/test/relations.test.js b/test/relations.test.js index 55a8e4cf..bc0a09d4 100644 --- a/test/relations.test.js +++ b/test/relations.test.js @@ -1805,6 +1805,23 @@ describe('relations', function () { }); }); + it('should remove items from scope - and save parent', function(done) { + Category.findById(category.id, function(err, cat) { + cat.items.at(0).destroy(function(err, link) { + cat.links.should.eql([]); + done(); + }); + }); + }); + + it('should find items on scope - verify destroy', function(done) { + Category.findById(category.id, function(err, cat) { + cat.name.should.equal('Category B'); + cat.links.should.eql([]); + done(); + }); + }); + }); describe('embedsMany - polymorphic relations', function () { From 0e348b0333b57e9e5f4a99fe8c8bc2d18f1f5994 Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Fri, 15 Aug 2014 18:51:19 +0200 Subject: [PATCH 28/34] Fixed duplicate code --- lib/relation-definition.js | 44 -------------------------------------- 1 file changed, 44 deletions(-) diff --git a/lib/relation-definition.js b/lib/relation-definition.js index c71c9184..e11b28bc 100644 --- a/lib/relation-definition.js +++ b/lib/relation-definition.js @@ -1696,50 +1696,6 @@ EmbedsMany.prototype.embeddedList = function(modelInstance) { return embeddedList; }; -EmbedsMany.prototype.prepareEmbeddedInstance = function(inst) { - if (inst && inst.triggerParent !== 'function') { - var self = this; - var relationName = this.definition.name; - var modelInstance = this.modelInstance; - inst.triggerParent = function(actionName, callback) { - if (actionName === 'save' || actionName === 'destroy') { - var embeddedList = self.embeddedList(); - if (actionName === 'destroy') { - var index = embeddedList.indexOf(inst); - if (index > -1) embeddedList.splice(index, 1); - } - modelInstance.updateAttribute(relationName, - embeddedList, function(err, modelInst) { - callback(err, err ? null : modelInst); - }); - } else { - process.nextTick(callback); - } - }; - var originalTrigger = inst.trigger; - inst.trigger = function(actionName, work, data, callback) { - if (typeof work === 'function') { - var originalWork = work; - work = function(next) { - originalWork.call(this, function(done) { - inst.triggerParent(actionName, function(err, inst) { - next(done); // TODO [fabien] - error handling? - }); - }); - }; - } - originalTrigger.call(this, actionName, work, data, callback); - }; - } -}; - -EmbedsMany.prototype.embeddedList = function(modelInstance) { - modelInstance = modelInstance || this.modelInstance; - var embeddedList = modelInstance[this.definition.name] || []; - embeddedList.forEach(this.prepareEmbeddedInstance.bind(this)); - return embeddedList; -}; - EmbedsMany.prototype.related = function(receiver, scopeParams, condOrRefresh, cb) { var modelTo = this.definition.modelTo; var modelInstance = this.modelInstance; From 085bb94505e49a85a1203f92771e21f60d098757 Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Thu, 14 Aug 2014 09:40:03 +0200 Subject: [PATCH 29/34] Allow partial list of ids for sortByIds --- lib/dao.js | 32 +++++++++++++++++++++----------- test/basic-querying.test.js | 27 +++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 11 deletions(-) diff --git a/lib/dao.js b/lib/dao.js index 00fbf18c..f2e55a26 100644 --- a/lib/dao.js +++ b/lib/dao.js @@ -351,19 +351,29 @@ DataAccessObject.sortByIds = function(ids, results) { return (typeof id === 'object') ? id.toString() : id; }); - results.sort(function(x, y) { - var idA = (typeof x[pk] === 'object') ? x[pk].toString() : x[pk]; - var idB = (typeof y[pk] === 'object') ? y[pk].toString() : y[pk]; - var a = ids.indexOf(idA); - var b = ids.indexOf(idB); - if (a === -1 || b === -1) return 1; // last - if (a !== b) { - if (a > b) return 1; - if (a < b) return -1; - } + var indexOf = function(x) { + var id = (typeof x[pk] === 'object') ? x[pk].toString() : x[pk]; + return ids.indexOf(id); + }; + + var heading = []; + var tailing = []; + + results.forEach(function(x) { + var idx = indexOf(x); + idx === -1 ? tailing.push(x) : heading.push(x); }); - return results; + heading.sort(function(x, y) { + var a = indexOf(x); + var b = indexOf(y); + if (a === -1 || b === -1) return 1; // last + if (a === b) return 0; + if (a > b) return 1; + if (a < b) return -1; + }); + + return heading.concat(tailing); }; function convertNullToNotFoundError(ctx, cb) { diff --git a/test/basic-querying.test.js b/test/basic-querying.test.js index dd26928a..d978fe88 100644 --- a/test/basic-querying.test.js +++ b/test/basic-querying.test.js @@ -87,6 +87,33 @@ describe('basic-querying', function () { done(); }); }); + + it('should sortByIds', function(done) { + User.find(function(err, users) { + sorted = User.sortByIds([6, 5, 4, 3, 2, 1], users); + var names = sorted.map(function(u) { return u.name; }); + names.should.eql(['f', 'e', 'd', 'c', 'b', 'a']); + done(); + }); + }); + + it('should sortByIds - partial ids (1)', function(done) { + User.find(function(err, users) { + sorted = User.sortByIds([6, 5, 4], users); + var names = sorted.map(function(u) { return u.name; }); + names.should.eql(['f', 'e', 'd', 'a', 'b', 'c']); + done(); + }); + }); + + it('should sortByIds - partial ids (2)', function(done) { + User.find(function(err, users) { + sorted = User.sortByIds([5, 3, 2], users); + var names = sorted.map(function(u) { return u.name; }); + names.should.eql(['e', 'c', 'b', 'a', 'd', 'f']); + done(); + }); + }); }); From c599de42cdd7f3c68158a742d313f73727e238e7 Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Thu, 14 Aug 2014 17:13:58 +0200 Subject: [PATCH 30/34] Remove redundant test --- test/basic-querying.test.js | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/test/basic-querying.test.js b/test/basic-querying.test.js index d978fe88..184335e1 100644 --- a/test/basic-querying.test.js +++ b/test/basic-querying.test.js @@ -97,16 +97,7 @@ describe('basic-querying', function () { }); }); - it('should sortByIds - partial ids (1)', function(done) { - User.find(function(err, users) { - sorted = User.sortByIds([6, 5, 4], users); - var names = sorted.map(function(u) { return u.name; }); - names.should.eql(['f', 'e', 'd', 'a', 'b', 'c']); - done(); - }); - }); - - it('should sortByIds - partial ids (2)', function(done) { + it('should sortByIds - partial ids', function(done) { User.find(function(err, users) { sorted = User.sortByIds([5, 3, 2], users); var names = sorted.map(function(u) { return u.name; }); From 9f1c5d9c378ce01bf73fa358e292e6902ec839e9 Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Fri, 15 Aug 2014 19:39:18 +0200 Subject: [PATCH 31/34] Moved DataAccessObject.sortByIds to utils.js --- lib/dao.js | 33 +------------------------------- lib/utils.js | 38 ++++++++++++++++++++++++++++++++++++- test/basic-querying.test.js | 18 ------------------ test/util.test.js | 26 +++++++++++++++++++++++++ 4 files changed, 64 insertions(+), 51 deletions(-) diff --git a/lib/dao.js b/lib/dao.js index f2e55a26..6595a576 100644 --- a/lib/dao.js +++ b/lib/dao.js @@ -341,41 +341,10 @@ DataAccessObject.findByIds = function(ids, cond, cb) { filter.where[pk] = { inq: ids }; mergeQuery(filter, cond || {}); this.find(filter, function(err, results) { - cb(err, err ? results : this.sortByIds(ids, results)); + cb(err, err ? results : utils.sortObjectsByIds(pk, ids, results)); }.bind(this)); }; -DataAccessObject.sortByIds = function(ids, results) { - var pk = this.dataSource.idName(this.modelName) || 'id'; - ids = ids.map(function(id) { - return (typeof id === 'object') ? id.toString() : id; - }); - - var indexOf = function(x) { - var id = (typeof x[pk] === 'object') ? x[pk].toString() : x[pk]; - return ids.indexOf(id); - }; - - var heading = []; - var tailing = []; - - results.forEach(function(x) { - var idx = indexOf(x); - idx === -1 ? tailing.push(x) : heading.push(x); - }); - - heading.sort(function(x, y) { - var a = indexOf(x); - var b = indexOf(y); - if (a === -1 || b === -1) return 1; // last - if (a === b) return 0; - if (a > b) return 1; - if (a < b) return -1; - }); - - return heading.concat(tailing); -}; - function convertNullToNotFoundError(ctx, cb) { if (ctx.result !== null) return cb(); diff --git a/lib/utils.js b/lib/utils.js index 520cf462..087cd615 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -6,6 +6,7 @@ exports.parseSettings = parseSettings; exports.mergeSettings = mergeSettings; exports.isPlainObject = isPlainObject; exports.defineCachedRelations = defineCachedRelations; +exports.sortObjectsByIds = sortObjectsByIds; var traverse = require('traverse'); @@ -203,4 +204,39 @@ function defineCachedRelations(obj) { function isPlainObject(obj) { return (typeof obj === 'object') && (obj !== null) && (obj.constructor === Object); -} \ No newline at end of file +} + + + +function sortObjectsByIds(idName, ids, objects) { + ids = ids.map(function(id) { + return (typeof id === 'object') ? id.toString() : id; + }); + + var indexOf = function(x) { + var isObj = (typeof x[idName] === 'object'); // ObjectID + var id = isObj ? x[idName].toString() : x[idName]; + return ids.indexOf(id); + }; + + var heading = []; + var tailing = []; + + objects.forEach(function(x) { + if (typeof x === 'object') { + var idx = indexOf(x); + idx === -1 ? tailing.push(x) : heading.push(x); + } + }); + + heading.sort(function(x, y) { + var a = indexOf(x); + var b = indexOf(y); + if (a === -1 || b === -1) return 1; // last + if (a === b) return 0; + if (a > b) return 1; + if (a < b) return -1; + }); + + return heading.concat(tailing); +}; diff --git a/test/basic-querying.test.js b/test/basic-querying.test.js index 184335e1..dd26928a 100644 --- a/test/basic-querying.test.js +++ b/test/basic-querying.test.js @@ -87,24 +87,6 @@ describe('basic-querying', function () { done(); }); }); - - it('should sortByIds', function(done) { - User.find(function(err, users) { - sorted = User.sortByIds([6, 5, 4, 3, 2, 1], users); - var names = sorted.map(function(u) { return u.name; }); - names.should.eql(['f', 'e', 'd', 'c', 'b', 'a']); - done(); - }); - }); - - it('should sortByIds - partial ids', function(done) { - User.find(function(err, users) { - sorted = User.sortByIds([5, 3, 2], users); - var names = sorted.map(function(u) { return u.name; }); - names.should.eql(['e', 'c', 'b', 'a', 'd', 'f']); - done(); - }); - }); }); diff --git a/test/util.test.js b/test/util.test.js index 81de4a75..37da6fe5 100644 --- a/test/util.test.js +++ b/test/util.test.js @@ -3,6 +3,7 @@ var utils = require('../lib/utils'); var fieldsToArray = utils.fieldsToArray; var removeUndefined = utils.removeUndefined; var mergeSettings = utils.mergeSettings; +var sortObjectsByIds = utils.sortObjectsByIds; describe('util.fieldsToArray', function () { it('Turn objects and strings into an array of fields to include when finding models', function () { @@ -185,4 +186,29 @@ describe('mergeSettings', function () { should.deepEqual(dst.acls, expected.acls, 'Merged settings should match the expectation'); }); +}); + +describe('sortObjectsByIds', function () { + + var items = [ + { id: 1, name: 'a' }, + { id: 2, name: 'b' }, + { id: 3, name: 'c' }, + { id: 4, name: 'd' }, + { id: 5, name: 'e' }, + { id: 6, name: 'f' } + ]; + + it('should sort', function() { + var sorted = sortObjectsByIds('id', [6, 5, 4, 3, 2, 1], items); + var names = sorted.map(function(u) { return u.name; }); + should.deepEqual(names, ['f', 'e', 'd', 'c', 'b', 'a']); + }); + + it('should sort - partial ids', function() { + var sorted =sortObjectsByIds('id', [5, 3, 2], items); + var names = sorted.map(function(u) { return u.name; }); + should.deepEqual(names, ['e', 'c', 'b', 'a', 'd', 'f']); + }); + }); \ No newline at end of file From 3e300c1f35000e3258666cc4dc25960e365a0755 Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Fri, 15 Aug 2014 19:47:12 +0200 Subject: [PATCH 32/34] Add strict flag to sortObjectsByIds When true, any object not in the ids will be discarded and a subset will be returned. --- lib/utils.js | 3 ++- test/util.test.js | 10 ++++++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/lib/utils.js b/lib/utils.js index 087cd615..6afb9129 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -208,7 +208,7 @@ function isPlainObject(obj) { -function sortObjectsByIds(idName, ids, objects) { +function sortObjectsByIds(idName, ids, objects, strict) { ids = ids.map(function(id) { return (typeof id === 'object') ? id.toString() : id; }); @@ -225,6 +225,7 @@ function sortObjectsByIds(idName, ids, objects) { objects.forEach(function(x) { if (typeof x === 'object') { var idx = indexOf(x); + if (strict && idx === -1) return; idx === -1 ? tailing.push(x) : heading.push(x); } }); diff --git a/test/util.test.js b/test/util.test.js index 37da6fe5..f933190a 100644 --- a/test/util.test.js +++ b/test/util.test.js @@ -206,9 +206,15 @@ describe('sortObjectsByIds', function () { }); it('should sort - partial ids', function() { - var sorted =sortObjectsByIds('id', [5, 3, 2], items); + var sorted = sortObjectsByIds('id', [5, 3, 2], items); var names = sorted.map(function(u) { return u.name; }); should.deepEqual(names, ['e', 'c', 'b', 'a', 'd', 'f']); }); + + it('should sort - strict', function() { + var sorted = sortObjectsByIds('id', [5, 3, 2], items, true); + var names = sorted.map(function(u) { return u.name; }); + should.deepEqual(names, ['e', 'c', 'b']); + }); -}); \ No newline at end of file +}); From f37941dfd6347fc5d3c36a8c280b4fc5706d1456 Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Fri, 15 Aug 2014 13:48:38 -0700 Subject: [PATCH 33/34] Fix the test cases to avoid hard-coded ids --- test/relations.test.js | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/test/relations.test.js b/test/relations.test.js index babd55b6..87610f84 100644 --- a/test/relations.test.js +++ b/test/relations.test.js @@ -1762,12 +1762,15 @@ describe('relations', function () { done(); }); }); - + + var productId; + it('should create items on scope', function(done) { Category.create({ name: 'Category B' }, function(err, cat) { category = cat; var link = cat.items.build({ notes: 'Some notes...' }); link.product.create({ name: 'Product 1' }, function(err, p) { + productId = p.id; cat.links[0].id.should.eql(p.id); cat.links[0].name.should.equal('Product 1'); // denormalized cat.links[0].notes.should.equal('Some notes...'); @@ -1781,7 +1784,7 @@ describe('relations', function () { Category.findById(category.id, function(err, cat) { cat.name.should.equal('Category B'); cat.links.toObject().should.eql([ - {id: 5, name: 'Product 1', notes: 'Some notes...'} + {id: productId, name: 'Product 1', notes: 'Some notes...'} ]); cat.items.at(0).should.equal(cat.links[0]); cat.items(function(err, items) { // alternative access @@ -1809,7 +1812,7 @@ describe('relations', function () { Category.findById(category.id, function(err, cat) { cat.name.should.equal('Category B'); cat.links.toObject().should.eql([ - {id: 5, name: 'Product 1', notes: 'Updated notes...'} + {id: productId, name: 'Product 1', notes: 'Updated notes...'} ]); done(); }); @@ -2239,7 +2242,8 @@ describe('relations', function () { }); describe('custom relation/scope methods', function () { - + var categoryId; + before(function (done) { db = getSchema(); Category = db.define('Category', {name: String}); @@ -2282,6 +2286,7 @@ describe('relations', function () { it('should setup test records', function (done) { Category.create({ name: 'Category A' }, function(err, cat) { + categoryId = cat.id; cat.products.create({ name: 'Product 1' }, function(err, p) { cat.products.create({ name: 'Product 2' }, function(err, p) { done(); @@ -2292,8 +2297,8 @@ describe('relations', function () { it('should allow custom scope methods - summarize', function(done) { var expected = [ - { name: 'Product 1', categoryId: 1, categoryName: 'Category A' }, - { name: 'Product 2', categoryId: 1, categoryName: 'Category A' } + { name: 'Product 1', categoryId: categoryId, categoryName: 'Category A' }, + { name: 'Product 2', categoryId: categoryId, categoryName: 'Category A' } ]; Category.findOne(function(err, cat) { From 5baf05c237aeb1ca15b01dd7b79b27333f8b0db0 Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Fri, 15 Aug 2014 13:49:09 -0700 Subject: [PATCH 34/34] Bump version --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index a4e4fc46..67e5064f 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "loopback-datasource-juggler", - "version": "2.3.1", + "version": "2.4.0", "description": "LoopBack DataSoure Juggler", "keywords": [ "StrongLoop",