From 4abe7f4954defe460bc1d994fde8a7165645ecb3 Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Thu, 4 Sep 2014 15:33:12 +0200 Subject: [PATCH 1/6] Isolate transient schema helper --- test/relations.test.js | 11 ++++++++--- test/transient.test.js | 7 ++++++- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/test/relations.test.js b/test/relations.test.js index 89607fc7..dc375017 100644 --- a/test/relations.test.js +++ b/test/relations.test.js @@ -1,5 +1,6 @@ // This test written in mocha+should.js var should = require('./init.js'); +var Schema = require('../').Schema; var db, tmp, Book, Chapter, Author, Reader; var Category, Job; @@ -7,6 +8,10 @@ var Picture, PictureLink; var Person, Address; var Link; +var getTransientSchema = function(settings) { + return new Schema('transient', settings); +}; + describe('relations', function () { describe('hasMany', function () { @@ -1679,7 +1684,7 @@ describe('relations', function () { var Other; before(function () { - tmp = getSchema('transient'); + tmp = getTransientSchema(); db = getSchema(); Person = db.define('Person', {name: String}); Passport = tmp.define('Passport', @@ -1828,7 +1833,7 @@ describe('relations', function () { var address1, address2; before(function (done) { - tmp = getSchema('transient', {defaultIdType: Number}); + tmp = getTransientSchema({defaultIdType: Number}); db = getSchema(); Person = db.define('Person', {name: String}); Address = tmp.define('Address', {street: String}); @@ -2008,7 +2013,7 @@ describe('relations', function () { describe('embedsMany - explicit ids', function () { before(function (done) { - tmp = getSchema('transient'); + tmp = getTransientSchema(); db = getSchema(); Person = db.define('Person', {name: String}); Address = tmp.define('Address', {street: String}); diff --git a/test/transient.test.js b/test/transient.test.js index ec713a9c..82b063cb 100644 --- a/test/transient.test.js +++ b/test/transient.test.js @@ -1,15 +1,20 @@ var jdb = require('../'); +var Schema = jdb.Schema; var DataSource = jdb.DataSource; var assert = require('assert'); var async = require('async'); var should = require('./init.js'); +var getTransientSchema = function(settings) { + return new Schema('transient', settings); +}; + var db, TransientModel, Person, Widget, Item; describe('Transient connector', function () { before(function () { - db = getSchema('transient'); + db = getTransientSchema(); TransientModel = db.define('TransientModel', {}, { idInjection: false }); Person = TransientModel.extend('Person', {name: String}); From 07dbbd4224bf8ea9eb269147eccdb71c104b1e97 Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Thu, 4 Sep 2014 15:37:48 +0200 Subject: [PATCH 2/6] Fix #283 --- lib/relation-definition.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/relation-definition.js b/lib/relation-definition.js index 3185a6a4..e32bd253 100644 --- a/lib/relation-definition.js +++ b/lib/relation-definition.js @@ -391,13 +391,13 @@ util.inherits(HasOne, Relation); * EmbedsOne subclass * @param {RelationDefinition|Object} definition * @param {Object} modelInstance - * @returns {EmbedsMany} + * @returns {EmbedsOne} * @constructor * @class EmbedsOne */ function EmbedsOne(definition, modelInstance) { if (!(this instanceof EmbedsOne)) { - return new EmbedsMany(definition, modelInstance); + return new EmbedsOne(definition, modelInstance); } assert(definition.type === RelationTypes.embedsOne); Relation.apply(this, arguments); From 2c0ffee2d3f088f25cbc6f3f861b53a64330c199 Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Thu, 4 Sep 2014 17:31:53 +0200 Subject: [PATCH 3/6] Polymorphic lookup from all registered dataSources Polymorphic model lookup was previously limited to the same dataSource as the modelFrom model, which turns out to be too restrictive. This was uncovered by the use of a Transient model, not being able to lookup a PersistedModel. --- index.js | 6 ++++++ lib/relation-definition.js | 34 +++++++++++++++++++++++----------- test/relations.test.js | 20 ++++++++++++++++---- 3 files changed, 45 insertions(+), 15 deletions(-) diff --git a/index.js b/index.js index a1e5420e..8f6c6d17 100644 --- a/index.js +++ b/index.js @@ -4,6 +4,12 @@ exports.ModelBaseClass = require('./lib/model.js'); exports.GeoPoint = require('./lib/geo.js').GeoPoint; exports.ValidationError = require('./lib/validations.js').ValidationError; +var dataSources = exports.dataSources = {}; + +exports.registerDataSource = function(ds) { + dataSources[ds.name] = ds; +}; + exports.__defineGetter__('version', function () { return require('./package.json').version; }); diff --git a/lib/relation-definition.js b/lib/relation-definition.js index e32bd253..818b5610 100644 --- a/lib/relation-definition.js +++ b/lib/relation-definition.js @@ -4,6 +4,7 @@ var assert = require('assert'); var util = require('util'); var i8n = require('inflection'); +var jdb = require('../'); var defineScope = require('./scope.js').defineScope; var mergeQuery = require('./scope.js').mergeQuery; var ModelBaseClass = require('./model.js'); @@ -466,7 +467,7 @@ function findBelongsTo(modelFrom, modelTo, keyTo) { * @param {String} modelName The model name * @returns {*} The matching model class */ -function lookupModel(models, modelName) { +function lookupModel(models, modelName, internal) { if(models[modelName]) { return models[modelName]; } @@ -476,6 +477,15 @@ function lookupModel(models, modelName) { return models[name]; } } + if (internal) return; + + var keys = Object.keys(jdb.dataSources); + for (var k = 0; k < keys.length; k++) { + var ds = jdb.dataSources[keys[k]]; + var models = ds.modelBuilder.models; + var model = lookupModel(models, modelName, true); + if (model) return model; + } } function lookupModelTo(modelFrom, modelTo, params, singularize) { @@ -503,9 +513,9 @@ function lookupModelTo(modelFrom, modelTo, params, singularize) { * @param {Object|String} params Name of the polymorphic relation or params * @returns {Object} The normalized parameters */ -function polymorphicParams(params) { +function polymorphicParams(params, as) { if (typeof params === 'string') params = { as: params }; - if (typeof params.as !== 'string') params.as = 'reference'; // default + if (typeof params.as !== 'string') params.as = as || 'reference'; // default params.foreignKey = params.foreignKey || i8n.camelize(params.as + '_id', true); params.discriminator = params.discriminator || i8n.camelize(params.as + '_type', true); return params; @@ -1079,21 +1089,23 @@ RelationDefinition.belongsTo = function (modelFrom, modelTo, params) { var idName, relationName, fk; if (params.polymorphic) { + relationName = params.as || (typeof modelTo === 'string' ? modelTo : null); // initially + if (params.polymorphic === true) { // modelTo arg will be the name of the polymorphic relation (string) - polymorphic = polymorphicParams(modelTo); + polymorphic = polymorphicParams(modelTo, relationName); } else { - polymorphic = polymorphicParams(params.polymorphic); + polymorphic = polymorphicParams(params.polymorphic, relationName); } modelTo = null; // will lookup dynamically idName = params.idName || 'id'; - relationName = params.as || polymorphic.as; + relationName = params.as || polymorphic.as; // finally fk = polymorphic.foreignKey; discriminator = polymorphic.discriminator; - if (typeof polymorphic.idType === 'string') { // explicit key type + if (polymorphic.idType) { // explicit key type modelFrom.dataSource.defineProperty(modelFrom.modelName, fk, { type: polymorphic.idType, index: true }); } else { // try to use the same foreign key type as modelFrom modelFrom.dataSource.defineForeignKey(modelFrom.modelName, fk, modelFrom.modelName); @@ -2208,10 +2220,7 @@ EmbedsMany.prototype.build = function(targetModelData) { var assignId = (forceId || targetModelData[pk] === undefined); - if (assignId && typeof connector.generateId === 'function') { - var id = connector.generateId(modelTo.modelName, targetModelData, pk); - targetModelData[pk] = id; - } else if (assignId && pkType === Number) { + if (assignId && pkType === Number) { var ids = embeddedList.map(function(m) { return (typeof m[pk] === 'number' ? m[pk] : 0); }); @@ -2220,6 +2229,9 @@ EmbedsMany.prototype.build = function(targetModelData) { } else { targetModelData[pk] = 1; } + } else if (assignId && typeof connector.generateId === 'function') { + var id = connector.generateId(modelTo.modelName, targetModelData, pk); + targetModelData[pk] = id; } this.definition.applyProperties(modelInstance, targetModelData); diff --git a/test/relations.test.js b/test/relations.test.js index dc375017..77619238 100644 --- a/test/relations.test.js +++ b/test/relations.test.js @@ -1,6 +1,7 @@ // This test written in mocha+should.js var should = require('./init.js'); -var Schema = require('../').Schema; +var jdb = require('../'); +var Schema = jdb.Schema; var db, tmp, Book, Chapter, Author, Reader; var Category, Job; @@ -2420,11 +2421,20 @@ describe('relations', function () { before(function (done) { db = getSchema(); + tmp = getTransientSchema(); + + // register here, so transient models + // can lookup related models (polymorphic) + jdb.registerDataSource(db); + Book = db.define('Book', {name: String}); Author = db.define('Author', {name: String}); Reader = db.define('Reader', {name: String}); - Link = db.define('Link', {name: String, notes: String}); // generic model + Link = tmp.define('Link', { + id: {type: Number, id: true}, + name: String, notes: String + }); // generic model Link.validatesPresenceOf('linkedId'); Link.validatesPresenceOf('linkedType'); @@ -2438,13 +2448,15 @@ describe('relations', function () { }); it('can be declared', function (done) { + var idType = db.connector.getDefaultIdType(); + Book.embedsMany(Link, { as: 'people', polymorphic: 'linked', scope: { include: 'linked' } }); Link.belongsTo('linked', { - polymorphic: true, // needs unique auto-id - properties: { name: 'name' }, // denormalized + polymorphic: { idType: idType }, // native type + properties: { name: 'name' }, // denormalized options: { invertProperties: true } }); db.automigrate(done); From 72930bf20b2781533523e3afbcafad3e114311f3 Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Thu, 4 Sep 2014 17:54:42 +0200 Subject: [PATCH 4/6] Re-use modelBuilder - correctly fixes lookup --- index.js | 6 ------ lib/relation-definition.js | 12 +----------- test/relations.test.js | 6 +----- test/transient.test.js | 4 ++-- 4 files changed, 4 insertions(+), 24 deletions(-) diff --git a/index.js b/index.js index 8f6c6d17..a1e5420e 100644 --- a/index.js +++ b/index.js @@ -4,12 +4,6 @@ exports.ModelBaseClass = require('./lib/model.js'); exports.GeoPoint = require('./lib/geo.js').GeoPoint; exports.ValidationError = require('./lib/validations.js').ValidationError; -var dataSources = exports.dataSources = {}; - -exports.registerDataSource = function(ds) { - dataSources[ds.name] = ds; -}; - exports.__defineGetter__('version', function () { return require('./package.json').version; }); diff --git a/lib/relation-definition.js b/lib/relation-definition.js index 818b5610..3a5b7371 100644 --- a/lib/relation-definition.js +++ b/lib/relation-definition.js @@ -4,7 +4,6 @@ var assert = require('assert'); var util = require('util'); var i8n = require('inflection'); -var jdb = require('../'); var defineScope = require('./scope.js').defineScope; var mergeQuery = require('./scope.js').mergeQuery; var ModelBaseClass = require('./model.js'); @@ -467,7 +466,7 @@ function findBelongsTo(modelFrom, modelTo, keyTo) { * @param {String} modelName The model name * @returns {*} The matching model class */ -function lookupModel(models, modelName, internal) { +function lookupModel(models, modelName) { if(models[modelName]) { return models[modelName]; } @@ -477,15 +476,6 @@ function lookupModel(models, modelName, internal) { return models[name]; } } - if (internal) return; - - var keys = Object.keys(jdb.dataSources); - for (var k = 0; k < keys.length; k++) { - var ds = jdb.dataSources[keys[k]]; - var models = ds.modelBuilder.models; - var model = lookupModel(models, modelName, true); - if (model) return model; - } } function lookupModelTo(modelFrom, modelTo, params, singularize) { diff --git a/test/relations.test.js b/test/relations.test.js index 77619238..eb042903 100644 --- a/test/relations.test.js +++ b/test/relations.test.js @@ -10,7 +10,7 @@ var Person, Address; var Link; var getTransientSchema = function(settings) { - return new Schema('transient', settings); + return new Schema('transient', settings, db.modelBuilder); }; describe('relations', function () { @@ -2423,10 +2423,6 @@ describe('relations', function () { db = getSchema(); tmp = getTransientSchema(); - // register here, so transient models - // can lookup related models (polymorphic) - jdb.registerDataSource(db); - Book = db.define('Book', {name: String}); Author = db.define('Author', {name: String}); Reader = db.define('Reader', {name: String}); diff --git a/test/transient.test.js b/test/transient.test.js index 82b063cb..89c1dd30 100644 --- a/test/transient.test.js +++ b/test/transient.test.js @@ -5,12 +5,12 @@ var assert = require('assert'); var async = require('async'); var should = require('./init.js'); +var db, TransientModel, Person, Widget, Item; + var getTransientSchema = function(settings) { return new Schema('transient', settings); }; -var db, TransientModel, Person, Widget, Item; - describe('Transient connector', function () { before(function () { From 7b788a290318b50246e90f6998a016b0c27b228d Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Thu, 4 Sep 2014 17:56:26 +0200 Subject: [PATCH 5/6] getTransientSchema => getTransientDataSource --- test/relations.test.js | 10 +++++----- test/transient.test.js | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/test/relations.test.js b/test/relations.test.js index eb042903..a7954846 100644 --- a/test/relations.test.js +++ b/test/relations.test.js @@ -9,7 +9,7 @@ var Picture, PictureLink; var Person, Address; var Link; -var getTransientSchema = function(settings) { +var getTransientDataSource = function(settings) { return new Schema('transient', settings, db.modelBuilder); }; @@ -1685,7 +1685,7 @@ describe('relations', function () { var Other; before(function () { - tmp = getTransientSchema(); + tmp = getTransientDataSource(); db = getSchema(); Person = db.define('Person', {name: String}); Passport = tmp.define('Passport', @@ -1834,7 +1834,7 @@ describe('relations', function () { var address1, address2; before(function (done) { - tmp = getTransientSchema({defaultIdType: Number}); + tmp = getTransientDataSource({defaultIdType: Number}); db = getSchema(); Person = db.define('Person', {name: String}); Address = tmp.define('Address', {street: String}); @@ -2014,7 +2014,7 @@ describe('relations', function () { describe('embedsMany - explicit ids', function () { before(function (done) { - tmp = getTransientSchema(); + tmp = getTransientDataSource(); db = getSchema(); Person = db.define('Person', {name: String}); Address = tmp.define('Address', {street: String}); @@ -2421,7 +2421,7 @@ describe('relations', function () { before(function (done) { db = getSchema(); - tmp = getTransientSchema(); + tmp = getTransientDataSource(); Book = db.define('Book', {name: String}); Author = db.define('Author', {name: String}); diff --git a/test/transient.test.js b/test/transient.test.js index 89c1dd30..d6ad1d3e 100644 --- a/test/transient.test.js +++ b/test/transient.test.js @@ -7,14 +7,14 @@ var should = require('./init.js'); var db, TransientModel, Person, Widget, Item; -var getTransientSchema = function(settings) { +var getTransientDataSource = function(settings) { return new Schema('transient', settings); }; describe('Transient connector', function () { before(function () { - db = getTransientSchema(); + db = getTransientDataSource(); TransientModel = db.define('TransientModel', {}, { idInjection: false }); Person = TransientModel.extend('Person', {name: String}); From 830a3f6f1a79304a50c54ddbef3c3eca33ddd76e Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Thu, 4 Sep 2014 18:01:04 +0200 Subject: [PATCH 6/6] Remove legacy Schema references --- test/relations.test.js | 4 ++-- test/transient.test.js | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/test/relations.test.js b/test/relations.test.js index a7954846..b99f31ba 100644 --- a/test/relations.test.js +++ b/test/relations.test.js @@ -1,7 +1,7 @@ // This test written in mocha+should.js var should = require('./init.js'); var jdb = require('../'); -var Schema = jdb.Schema; +var DataSource = jdb.DataSource; var db, tmp, Book, Chapter, Author, Reader; var Category, Job; @@ -10,7 +10,7 @@ var Person, Address; var Link; var getTransientDataSource = function(settings) { - return new Schema('transient', settings, db.modelBuilder); + return new DataSource('transient', settings, db.modelBuilder); }; describe('relations', function () { diff --git a/test/transient.test.js b/test/transient.test.js index d6ad1d3e..e3a03f96 100644 --- a/test/transient.test.js +++ b/test/transient.test.js @@ -1,5 +1,4 @@ var jdb = require('../'); -var Schema = jdb.Schema; var DataSource = jdb.DataSource; var assert = require('assert'); var async = require('async'); @@ -8,7 +7,7 @@ var should = require('./init.js'); var db, TransientModel, Person, Widget, Item; var getTransientDataSource = function(settings) { - return new Schema('transient', settings); + return new DataSource('transient', settings); }; describe('Transient connector', function () {