From a9f2da294c411c17e538967682e3615b83b56416 Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Sat, 16 Aug 2014 10:23:32 +0200 Subject: [PATCH 1/8] Properly handle LDL for polymorphic relations --- lib/datasource.js | 27 +++++++++++++++++++--- lib/relation-definition.js | 26 ++++++++++++++++----- test/loopback-dl.test.js | 46 +++++++++++++++++++++++++++++++++++++- test/relations.test.js | 31 +++++++++++++++++++++++++ 4 files changed, 121 insertions(+), 9 deletions(-) diff --git a/lib/datasource.js b/lib/datasource.js index 9602ec4f..fefe10a2 100644 --- a/lib/datasource.js +++ b/lib/datasource.js @@ -14,6 +14,7 @@ var util = require('util'); var assert = require('assert'); var async = require('async'); var traverse = require('traverse'); +var i8n = require('inflection'); if (process.env.DEBUG === 'loopback') { // For back-compatibility @@ -429,19 +430,39 @@ DataSource.prototype.defineRelations = function (modelClass, relations) { for (var rn in relations) { var r = relations[rn]; assert(DataSource.relationTypes.indexOf(r.type) !== -1, "Invalid relation type: " + r.type); - var targetModel = isModelClass(r.model) ? r.model : this.getModel(r.model, true); + var targetModel, polymorphicName; + + if (r.polymorphic && r.type !== 'belongsTo' && !r.model) { + throw new Error('No model specified for polymorphic ' + r.type + ': ' + rn); + } + + if (r.polymorphic) { + polymorphicName = typeof r.model === 'string' ? r.model : rn; + if (typeof r.polymorphic === 'string') { + polymorphicName = r.polymorphic; + } else if (typeof r.polymorphic === 'object' && typeof r.polymorphic.as === 'string') { + polymorphicName = r.polymorphic.as; + } + } + + if (r.model) { + targetModel = isModelClass(r.model) ? r.model : this.getModel(r.model, true); + } + var throughModel = null; if (r.through) { throughModel = isModelClass(r.through) ? r.through : this.getModel(r.through, true); } - if ((!r.polymorphic && !isModelDataSourceAttached(targetModel)) || (throughModel && !isModelDataSourceAttached(throughModel))) { + + if ((targetModel && !isModelDataSourceAttached(targetModel)) + || (throughModel && !isModelDataSourceAttached(throughModel))) { // Create a listener to defer the relation set up createListener(rn, r, targetModel, throughModel); } else { // The target model is resolved var params = traverse(r).clone(); params.as = rn; - params.model = targetModel; + params.model = polymorphicName || targetModel; if (throughModel) { params.through = throughModel; } diff --git a/lib/relation-definition.js b/lib/relation-definition.js index e11b28bc..c1b51ddb 100644 --- a/lib/relation-definition.js +++ b/lib/relation-definition.js @@ -115,12 +115,19 @@ function RelationDefinition(definition) { } RelationDefinition.prototype.toJSON = function () { + var polymorphic = typeof this.polymorphic === 'object'; + + var modelToName = this.modelTo && this.modelTo.modelName; + if (!modelToName && polymorphic && this.type === 'belongsTo') { + modelToName = ''; + } + var json = { name: this.name, type: this.type, modelFrom: this.modelFrom.modelName, keyFrom: this.keyFrom, - modelTo: this.modelTo.modelName, + modelTo: modelToName, keyTo: this.keyTo, multiple: this.multiple }; @@ -128,6 +135,9 @@ RelationDefinition.prototype.toJSON = function () { json.modelThrough = this.modelThrough.modelName; json.keyThrough = this.keyThrough; } + if (polymorphic) { + json.polymorphic = this.polymorphic; + } return json; }; @@ -449,14 +459,20 @@ function lookupModel(models, modelName) { function lookupModelTo(modelFrom, modelTo, params, singularize) { if ('string' === typeof modelTo) { + var modelToName; params.as = params.as || modelTo; modelTo = params.model || modelTo; if (typeof modelTo === 'string') { - var modelToName = (singularize ? i8n.singularize(modelTo) : modelTo).toLowerCase(); + modelToName = (singularize ? i8n.singularize(modelTo) : modelTo).toLowerCase(); + modelTo = lookupModel(modelFrom.dataSource.modelBuilder.models, modelToName) || modelTo; + } + if (typeof modelTo === 'string') { + modelToName = (singularize ? i8n.singularize(params.as) : params.as).toLowerCase(); + console.log(modelToName) modelTo = lookupModel(modelFrom.dataSource.modelBuilder.models, modelToName) || modelTo; } if (typeof modelTo !== 'function') { - throw new Error('Could not find "' + modelTo + '" relation for ' + modelFrom.modelName); + throw new Error('Could not find "' + params.as + '" relation for ' + modelFrom.modelName); } } return modelTo; @@ -510,7 +526,7 @@ RelationDefinition.hasMany = function hasMany(modelFrom, modelTo, params) { if (params.polymorphic) { polymorphic = polymorphicParams(params.polymorphic); - polymorphic.invert = !!params.invert; + if (params.invert) polymorphic.invert = true; discriminator = polymorphic.discriminator; if (!params.invert) { fk = polymorphic.foreignKey; @@ -1172,7 +1188,7 @@ BelongsTo.prototype.related = function (refresh, params) { self.resetCache(params); } else if (typeof params === 'function') { // acts as async getter - if (discriminator && !modelTo) { + if (discriminator) { var modelToName = modelInstance[discriminator]; if (typeof modelToName !== 'string') { throw new Error('Polymorphic model not found: `' + discriminator + '` not set'); diff --git a/test/loopback-dl.test.js b/test/loopback-dl.test.js index 458f91db..9d2d42d2 100644 --- a/test/loopback-dl.test.js +++ b/test/loopback-dl.test.js @@ -923,7 +923,51 @@ describe('Load models with relations', function () { assert(User.relations['posts']); done(); }); - + + it('should set up polymorphic relations', function (done) { + var ds = new DataSource('memory'); + + var Author = ds.define('Author', {name: String}, {relations: { + pictures: {type: 'hasMany', model: 'Picture', polymorphic: 'imageable'} + }}); + var Picture = ds.define('Picture', {name: String}, {relations: { + imageable: {type: 'belongsTo', polymorphic: true} + }}); + + assert(Author.relations['pictures']); + assert.deepEqual(Author.relations['pictures'].toJSON(), { + name: 'pictures', + type: 'hasMany', + modelFrom: 'Author', + keyFrom: 'id', + modelTo: 'Picture', + keyTo: 'imageableId', + multiple: true, + polymorphic: { + as: 'imageable', + foreignKey: 'imageableId', + discriminator: 'imageableType' + } + }); + + assert(Picture.relations['imageable']); + assert.deepEqual(Picture.relations['imageable'].toJSON(), { + name: 'imageable', + type: 'belongsTo', + modelFrom: 'Picture', + keyFrom: 'imageableId', + modelTo: '', + keyTo: 'id', + multiple: false, + polymorphic: { + as: 'imageable', + foreignKey: 'imageableId', + discriminator: 'imageableType' + } + }); + done(); + }); + it('should set up foreign key with the correct type', function (done) { var ds = new DataSource('memory'); diff --git a/test/relations.test.js b/test/relations.test.js index 87610f84..871ecb54 100644 --- a/test/relations.test.js +++ b/test/relations.test.js @@ -686,6 +686,37 @@ describe('relations', function () { discriminator: 'imageableType' } }); Picture.belongsTo('imageable', { polymorphic: true }); + + Author.relations['pictures'].toJSON().should.eql({ + name: 'pictures', + type: 'hasMany', + modelFrom: 'Author', + keyFrom: 'id', + modelTo: 'Picture', + keyTo: 'imageableId', + multiple: true, + polymorphic: { + as: 'imageable', + foreignKey: 'imageableId', + discriminator: 'imageableType' + } + }); + + Picture.relations['imageable'].toJSON().should.eql({ + name: 'imageable', + type: 'belongsTo', + modelFrom: 'Picture', + keyFrom: 'imageableId', + modelTo: '', + keyTo: 'id', + multiple: false, + polymorphic: { + as: 'imageable', + foreignKey: 'imageableId', + discriminator: 'imageableType' + } + }); + db.automigrate(done); }); From ad780604e155e7eb9e276796f7c0f855a24e1894 Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Sat, 16 Aug 2014 10:23:32 +0200 Subject: [PATCH 2/8] Properly handle LDL for polymorphic relations --- lib/datasource.js | 27 +++++++++++++++++++--- lib/relation-definition.js | 26 ++++++++++++++++----- test/loopback-dl.test.js | 46 +++++++++++++++++++++++++++++++++++++- test/relations.test.js | 31 +++++++++++++++++++++++++ 4 files changed, 121 insertions(+), 9 deletions(-) diff --git a/lib/datasource.js b/lib/datasource.js index 9602ec4f..fefe10a2 100644 --- a/lib/datasource.js +++ b/lib/datasource.js @@ -14,6 +14,7 @@ var util = require('util'); var assert = require('assert'); var async = require('async'); var traverse = require('traverse'); +var i8n = require('inflection'); if (process.env.DEBUG === 'loopback') { // For back-compatibility @@ -429,19 +430,39 @@ DataSource.prototype.defineRelations = function (modelClass, relations) { for (var rn in relations) { var r = relations[rn]; assert(DataSource.relationTypes.indexOf(r.type) !== -1, "Invalid relation type: " + r.type); - var targetModel = isModelClass(r.model) ? r.model : this.getModel(r.model, true); + var targetModel, polymorphicName; + + if (r.polymorphic && r.type !== 'belongsTo' && !r.model) { + throw new Error('No model specified for polymorphic ' + r.type + ': ' + rn); + } + + if (r.polymorphic) { + polymorphicName = typeof r.model === 'string' ? r.model : rn; + if (typeof r.polymorphic === 'string') { + polymorphicName = r.polymorphic; + } else if (typeof r.polymorphic === 'object' && typeof r.polymorphic.as === 'string') { + polymorphicName = r.polymorphic.as; + } + } + + if (r.model) { + targetModel = isModelClass(r.model) ? r.model : this.getModel(r.model, true); + } + var throughModel = null; if (r.through) { throughModel = isModelClass(r.through) ? r.through : this.getModel(r.through, true); } - if ((!r.polymorphic && !isModelDataSourceAttached(targetModel)) || (throughModel && !isModelDataSourceAttached(throughModel))) { + + if ((targetModel && !isModelDataSourceAttached(targetModel)) + || (throughModel && !isModelDataSourceAttached(throughModel))) { // Create a listener to defer the relation set up createListener(rn, r, targetModel, throughModel); } else { // The target model is resolved var params = traverse(r).clone(); params.as = rn; - params.model = targetModel; + params.model = polymorphicName || targetModel; if (throughModel) { params.through = throughModel; } diff --git a/lib/relation-definition.js b/lib/relation-definition.js index e11b28bc..c1b51ddb 100644 --- a/lib/relation-definition.js +++ b/lib/relation-definition.js @@ -115,12 +115,19 @@ function RelationDefinition(definition) { } RelationDefinition.prototype.toJSON = function () { + var polymorphic = typeof this.polymorphic === 'object'; + + var modelToName = this.modelTo && this.modelTo.modelName; + if (!modelToName && polymorphic && this.type === 'belongsTo') { + modelToName = ''; + } + var json = { name: this.name, type: this.type, modelFrom: this.modelFrom.modelName, keyFrom: this.keyFrom, - modelTo: this.modelTo.modelName, + modelTo: modelToName, keyTo: this.keyTo, multiple: this.multiple }; @@ -128,6 +135,9 @@ RelationDefinition.prototype.toJSON = function () { json.modelThrough = this.modelThrough.modelName; json.keyThrough = this.keyThrough; } + if (polymorphic) { + json.polymorphic = this.polymorphic; + } return json; }; @@ -449,14 +459,20 @@ function lookupModel(models, modelName) { function lookupModelTo(modelFrom, modelTo, params, singularize) { if ('string' === typeof modelTo) { + var modelToName; params.as = params.as || modelTo; modelTo = params.model || modelTo; if (typeof modelTo === 'string') { - var modelToName = (singularize ? i8n.singularize(modelTo) : modelTo).toLowerCase(); + modelToName = (singularize ? i8n.singularize(modelTo) : modelTo).toLowerCase(); + modelTo = lookupModel(modelFrom.dataSource.modelBuilder.models, modelToName) || modelTo; + } + if (typeof modelTo === 'string') { + modelToName = (singularize ? i8n.singularize(params.as) : params.as).toLowerCase(); + console.log(modelToName) modelTo = lookupModel(modelFrom.dataSource.modelBuilder.models, modelToName) || modelTo; } if (typeof modelTo !== 'function') { - throw new Error('Could not find "' + modelTo + '" relation for ' + modelFrom.modelName); + throw new Error('Could not find "' + params.as + '" relation for ' + modelFrom.modelName); } } return modelTo; @@ -510,7 +526,7 @@ RelationDefinition.hasMany = function hasMany(modelFrom, modelTo, params) { if (params.polymorphic) { polymorphic = polymorphicParams(params.polymorphic); - polymorphic.invert = !!params.invert; + if (params.invert) polymorphic.invert = true; discriminator = polymorphic.discriminator; if (!params.invert) { fk = polymorphic.foreignKey; @@ -1172,7 +1188,7 @@ BelongsTo.prototype.related = function (refresh, params) { self.resetCache(params); } else if (typeof params === 'function') { // acts as async getter - if (discriminator && !modelTo) { + if (discriminator) { var modelToName = modelInstance[discriminator]; if (typeof modelToName !== 'string') { throw new Error('Polymorphic model not found: `' + discriminator + '` not set'); diff --git a/test/loopback-dl.test.js b/test/loopback-dl.test.js index 458f91db..9d2d42d2 100644 --- a/test/loopback-dl.test.js +++ b/test/loopback-dl.test.js @@ -923,7 +923,51 @@ describe('Load models with relations', function () { assert(User.relations['posts']); done(); }); - + + it('should set up polymorphic relations', function (done) { + var ds = new DataSource('memory'); + + var Author = ds.define('Author', {name: String}, {relations: { + pictures: {type: 'hasMany', model: 'Picture', polymorphic: 'imageable'} + }}); + var Picture = ds.define('Picture', {name: String}, {relations: { + imageable: {type: 'belongsTo', polymorphic: true} + }}); + + assert(Author.relations['pictures']); + assert.deepEqual(Author.relations['pictures'].toJSON(), { + name: 'pictures', + type: 'hasMany', + modelFrom: 'Author', + keyFrom: 'id', + modelTo: 'Picture', + keyTo: 'imageableId', + multiple: true, + polymorphic: { + as: 'imageable', + foreignKey: 'imageableId', + discriminator: 'imageableType' + } + }); + + assert(Picture.relations['imageable']); + assert.deepEqual(Picture.relations['imageable'].toJSON(), { + name: 'imageable', + type: 'belongsTo', + modelFrom: 'Picture', + keyFrom: 'imageableId', + modelTo: '', + keyTo: 'id', + multiple: false, + polymorphic: { + as: 'imageable', + foreignKey: 'imageableId', + discriminator: 'imageableType' + } + }); + done(); + }); + it('should set up foreign key with the correct type', function (done) { var ds = new DataSource('memory'); diff --git a/test/relations.test.js b/test/relations.test.js index 87610f84..871ecb54 100644 --- a/test/relations.test.js +++ b/test/relations.test.js @@ -686,6 +686,37 @@ describe('relations', function () { discriminator: 'imageableType' } }); Picture.belongsTo('imageable', { polymorphic: true }); + + Author.relations['pictures'].toJSON().should.eql({ + name: 'pictures', + type: 'hasMany', + modelFrom: 'Author', + keyFrom: 'id', + modelTo: 'Picture', + keyTo: 'imageableId', + multiple: true, + polymorphic: { + as: 'imageable', + foreignKey: 'imageableId', + discriminator: 'imageableType' + } + }); + + Picture.relations['imageable'].toJSON().should.eql({ + name: 'imageable', + type: 'belongsTo', + modelFrom: 'Picture', + keyFrom: 'imageableId', + modelTo: '', + keyTo: 'id', + multiple: false, + polymorphic: { + as: 'imageable', + foreignKey: 'imageableId', + discriminator: 'imageableType' + } + }); + db.automigrate(done); }); From 315d5c15c653b5515b65ceb95ac43395100b79f8 Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Sat, 16 Aug 2014 11:07:48 +0200 Subject: [PATCH 3/8] Tiny fix: obsolete i8n require --- lib/datasource.js | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/datasource.js b/lib/datasource.js index fefe10a2..36be539d 100644 --- a/lib/datasource.js +++ b/lib/datasource.js @@ -14,7 +14,6 @@ var util = require('util'); var assert = require('assert'); var async = require('async'); var traverse = require('traverse'); -var i8n = require('inflection'); if (process.env.DEBUG === 'loopback') { // For back-compatibility From b4b1c784dda213825f57b35012ce94108538a8cd Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Sat, 16 Aug 2014 13:04:13 +0200 Subject: [PATCH 4/8] HasMany exists should use internal findById This limits the scope correctly, taking polymorphics into account. (the foreign key check is actually obsolete I think) --- lib/relation-definition.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/relation-definition.js b/lib/relation-definition.js index e11b28bc..304ed782 100644 --- a/lib/relation-definition.js +++ b/lib/relation-definition.js @@ -692,12 +692,11 @@ HasMany.prototype.findById = function (fkId, cb) { * @param {Function} cb The callback function */ HasMany.prototype.exists = function (fkId, cb) { - var modelTo = this.definition.modelTo; var fk = this.definition.keyTo; var pk = this.definition.keyFrom; var modelInstance = this.modelInstance; - modelTo.findById(fkId, function (err, inst) { + this.findById(fkId, function (err, inst) { if (err) { return cb(err); } From 39a728be8426962b6881657c5a9376dc967acffa Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Sun, 17 Aug 2014 12:29:04 +0200 Subject: [PATCH 5/8] Add ability to apply a plugin multiple times from LDL --- lib/model-builder.js | 9 ++++++++- test/mixins.test.js | 17 +++++++++++++++-- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/lib/model-builder.js b/lib/model-builder.js index 708d328b..df46130e 100644 --- a/lib/model-builder.js +++ b/lib/model-builder.js @@ -439,7 +439,14 @@ ModelBuilder.prototype.define = function defineClass(className, properties, sett if (mixin === true) { mixin = {}; } - if (typeof mixin === 'object') { + if (Array.isArray(mixin)) { + mixin.forEach(function(m) { + if (m === true) m = {}; + if (typeof m === 'object') { + modelBuilder.mixins.applyMixin(ModelClass, name, m); + } + }); + } else if (typeof mixin === 'object') { modelBuilder.mixins.applyMixin(ModelClass, name, mixin); } } diff --git a/test/mixins.test.js b/test/mixins.test.js index e60f7e02..49a1634a 100644 --- a/test/mixins.test.js +++ b/test/mixins.test.js @@ -49,7 +49,11 @@ describe('Model class', function () { }; }); mixins.define('Demo', function(Model, options) { - Model.demoMixin = options.ok; + Model.demoMixin = options.value; + }); + mixins.define('Multi', function(Model, options) { + Model.multiMixin = Model.multiMixin || {}; + Model.multiMixin[options.key] = options.value; }); }); @@ -73,13 +77,22 @@ 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: { value: true }, + Multi: [ + { key: 'foo', value: 'bar' }, + { key: 'fox', value: 'baz' } + ] + } }); Item.mixin('Example', { foo: 'bar' }); Item.demoMixin.should.be.true; + Item.multiMixin.foo.should.equal('bar'); + Item.multiMixin.fox.should.equal('baz'); + var properties = Item.definition.properties; properties.createdAt.should.eql({ type: Date }); properties.updatedAt.should.eql({ type: Date }); From 291e7b2c7452b5bc17bf86206ec0d06809640279 Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Sun, 17 Aug 2014 18:01:52 +0200 Subject: [PATCH 6/8] Fix ModelDefinition toJSON bug --- lib/model-definition.js | 2 +- test/model-definition.test.js | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/model-definition.js b/lib/model-definition.js index 3a75ac08..ae53a1be 100644 --- a/lib/model-definition.js +++ b/lib/model-definition.js @@ -265,7 +265,7 @@ ModelDefinition.prototype.toJSON = function (forceRebuild) { this.json = null; } if (this.json) { - return json; + return this.json; } var json = { name: this.name, diff --git a/test/model-definition.test.js b/test/model-definition.test.js index 562e236e..080685e4 100644 --- a/test/model-definition.test.js +++ b/test/model-definition.test.js @@ -36,7 +36,9 @@ describe('ModelDefinition class', function () { assert.equal(json.properties.approved.type, "Boolean"); assert.equal(json.properties.joinedAt.type, "Date"); assert.equal(json.properties.age.type, "Number"); - + + assert.deepEqual(User.toJSON(), json); + done(); }); From dad82668379d0c481775264a9201873dc1f8e459 Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Mon, 18 Aug 2014 07:28:33 +0200 Subject: [PATCH 7/8] Prevent failure with null in List toObject --- lib/list.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/list.js b/lib/list.js index 4c4158eb..27c2a534 100644 --- a/lib/list.js +++ b/lib/list.js @@ -75,7 +75,7 @@ List.prototype.push = function (obj) { List.prototype.toObject = function (onlySchema, removeHidden) { var items = []; this.forEach(function (item) { - if (item.toObject) { + if (item && typeof item === 'object' && item.toObject) { items.push(item.toObject(onlySchema, removeHidden)); } else { items.push(item); From b47197e8547757e16974dcbdeaae28175f71b3d2 Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Mon, 18 Aug 2014 20:36:45 -0700 Subject: [PATCH 8/8] Bump version --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 96456bfc..7e75fb17 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "loopback-datasource-juggler", - "version": "2.4.1", + "version": "2.4.2", "description": "LoopBack DataSoure Juggler", "keywords": [ "StrongLoop",