From a953ba13de5d0ec979ccd026961d0a49240460fa Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Tue, 3 Dec 2013 21:14:12 -0800 Subject: [PATCH 1/5] Clone shared methods so that they can be customized per model --- lib/datasource.js | 22 ++++++++++++---------- lib/jutil.js | 16 ++++++++++++++++ test/loopback-dl.test.js | 20 ++++++++++++++++++++ 3 files changed, 48 insertions(+), 10 deletions(-) diff --git a/lib/datasource.js b/lib/datasource.js index dda360e9..5cea8682 100644 --- a/lib/datasource.js +++ b/lib/datasource.js @@ -90,14 +90,14 @@ function DataSource(name, settings, modelBuilder) { var connector = this.connector; // DataAccessObject - connector defined or supply the default - this.DataAccessObject = (connector && connector.DataAccessObject) ? connector.DataAccessObject : this.constructor.DataAccessObject; - this.DataAccessObject.apply(this, arguments); - + var dao = (connector && connector.DataAccessObject) || this.constructor.DataAccessObject; + this.DataAccessObject = function() {}; // define DataAccessObject methods - Object.keys(this.DataAccessObject).forEach(function (name) { - var fn = this.DataAccessObject[name]; - + Object.keys(dao).forEach(function (name) { + var fn = dao[name]; + this.DataAccessObject[name] = fn; + if(typeof fn === 'function') { this.defineOperation(name, { accepts: fn.accepts, @@ -111,11 +111,10 @@ function DataSource(name, settings, modelBuilder) { }.bind(this)); // define DataAccessObject.prototype methods - Object.keys(this.DataAccessObject.prototype).forEach(function (name) { - var fn = this.DataAccessObject.prototype[name]; - + Object.keys(dao.prototype).forEach(function (name) { + var fn = dao.prototype[name]; + this.DataAccessObject.prototype[name] = fn; if(typeof fn === 'function') { - this.defineOperation(name, { prototype: true, accepts: fn.accepts, @@ -127,8 +126,11 @@ function DataSource(name, settings, modelBuilder) { }); } }.bind(this)); + } + + util.inherits(DataSource, EventEmitter); // allow child classes to supply a data access object diff --git a/lib/jutil.js b/lib/jutil.js index 198b65e8..a74f254f 100644 --- a/lib/jutil.js +++ b/lib/jutil.js @@ -49,6 +49,9 @@ exports.mixin = function (newClass, mixinClass, options) { Object.keys(mixinClass).forEach(function (classProp) { if (classProp !== 'super_' && classProp !== '_mixins' && (!newClass.hasOwnProperty(classProp) || options.override)) { var pd = Object.getOwnPropertyDescriptor(mixinClass, classProp); + if(pd.writable && typeof pd.value === 'function' && pd.value.shared) { + pd.value = exports.proxy(pd.value); + } Object.defineProperty(newClass, classProp, pd); } }); @@ -59,6 +62,9 @@ exports.mixin = function (newClass, mixinClass, options) { Object.keys(mixinClass.prototype).forEach(function (instanceProp) { if (!newClass.prototype.hasOwnProperty(instanceProp) || options.override) { var pd = Object.getOwnPropertyDescriptor(mixinClass.prototype, instanceProp); + if(pd.writable && typeof pd.value === 'function' && pd.value.shared) { + pd.value = exports.proxy(pd.value); + } Object.defineProperty(newClass.prototype, instanceProp, pd); } }); @@ -68,3 +74,13 @@ exports.mixin = function (newClass, mixinClass, options) { return newClass; }; +exports.proxy = function(fn) { + var f = function() { + return fn.apply(this, arguments); + }; + Object.keys(fn).forEach(function(x) { + f[x] = fn[x]; + }); + return f; +}; + diff --git a/test/loopback-dl.test.js b/test/loopback-dl.test.js index fdad6fbb..4b50187d 100644 --- a/test/loopback-dl.test.js +++ b/test/loopback-dl.test.js @@ -673,3 +673,23 @@ describe('DataSource constructor', function(){ }); }); + +describe('Injected remotable methods', function(){ + it('are not shared across models', function() { + var ds = new DataSource('memory'); + var M1 = ds.createModel('M1'); + var M2 = ds.createModel('M2'); + // Remotable methods are not shared across models + assert.notEqual(M1.create, M2.create, 'Remotable methods are not shared'); + assert.equal(M1.create.shared, true, 'M1.create is remotable'); + assert.equal(M2.create.shared, true, 'M2.create is remotable'); + M1.create.shared = false; + assert.equal(M1.create.shared, false, 'M1.create should be local now'); + assert.equal(M2.create.shared, true, 'M2.create should stay remotable'); + // Non-shared method should be same + assert.equal(M1.prototype.save, M2.prototype.save, + 'Local methods should be the same'); + }); + + +}); From 52d2c8425f569820aa7db513248e2ada3746ecd3 Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Wed, 4 Dec 2013 13:44:25 -0800 Subject: [PATCH 2/5] Make all methods proxied for DAO --- lib/datasource.js | 2 +- lib/jutil.js | 15 ++++++++++++--- test/loopback-dl.test.js | 5 ++--- 3 files changed, 15 insertions(+), 7 deletions(-) diff --git a/lib/datasource.js b/lib/datasource.js index 5cea8682..84adf376 100644 --- a/lib/datasource.js +++ b/lib/datasource.js @@ -503,7 +503,7 @@ DataSource.prototype.mixin = function (ModelCtor) { var DAO = this.DataAccessObject; // mixin DAO - jutil.mixin(ModelCtor, DAO); + jutil.mixin(ModelCtor, DAO, {proxyFunctions : true}); // decorate operations as alias functions Object.keys(ops).forEach(function (name) { diff --git a/lib/jutil.js b/lib/jutil.js index a74f254f..6daa1134 100644 --- a/lib/jutil.js +++ b/lib/jutil.js @@ -42,14 +42,23 @@ exports.mixin = function (newClass, mixinClass, options) { options = options || { staticProperties: true, instanceProperties: true, - override: false + override: false, + proxyFunctions: false }; + if(options.staticProperties === undefined) { + options.staticProperties = true; + } + + if(options.instanceProperties === undefined) { + options.instanceProperties = true; + } + if (options.staticProperties) { Object.keys(mixinClass).forEach(function (classProp) { if (classProp !== 'super_' && classProp !== '_mixins' && (!newClass.hasOwnProperty(classProp) || options.override)) { var pd = Object.getOwnPropertyDescriptor(mixinClass, classProp); - if(pd.writable && typeof pd.value === 'function' && pd.value.shared) { + if(options.proxyFunctions && pd.writable && typeof pd.value === 'function') { pd.value = exports.proxy(pd.value); } Object.defineProperty(newClass, classProp, pd); @@ -62,7 +71,7 @@ exports.mixin = function (newClass, mixinClass, options) { Object.keys(mixinClass.prototype).forEach(function (instanceProp) { if (!newClass.prototype.hasOwnProperty(instanceProp) || options.override) { var pd = Object.getOwnPropertyDescriptor(mixinClass.prototype, instanceProp); - if(pd.writable && typeof pd.value === 'function' && pd.value.shared) { + if(options.proxyFunctions && pd.writable && typeof pd.value === 'function') { pd.value = exports.proxy(pd.value); } Object.defineProperty(newClass.prototype, instanceProp, pd); diff --git a/test/loopback-dl.test.js b/test/loopback-dl.test.js index 4b50187d..62be6a4f 100644 --- a/test/loopback-dl.test.js +++ b/test/loopback-dl.test.js @@ -686,9 +686,8 @@ describe('Injected remotable methods', function(){ M1.create.shared = false; assert.equal(M1.create.shared, false, 'M1.create should be local now'); assert.equal(M2.create.shared, true, 'M2.create should stay remotable'); - // Non-shared method should be same - assert.equal(M1.prototype.save, M2.prototype.save, - 'Local methods should be the same'); + assert.notEqual(M1.prototype.save, M2.prototype.save, + 'non-remote methods are not shared'); }); From 8360576c40f6ec3425ff2d1b303dccfde8670006 Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Wed, 4 Dec 2013 15:24:53 -0800 Subject: [PATCH 3/5] Attach models to the data source --- lib/datasource.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/datasource.js b/lib/datasource.js index dda360e9..62e1a688 100644 --- a/lib/datasource.js +++ b/lib/datasource.js @@ -1241,6 +1241,10 @@ DataSource.prototype.discoverAndBuildModels = function (modelName, options, cb) } var models = self.modelBuilder.buildModels(schemaList); + // Now attach the models to the data source + for(var m in models) { + models[m].attachTo(self); + } cb && cb(err, models); }); }; From e5824356ca1319f64698ddaaaec993cb3f2d9cb5 Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Wed, 4 Dec 2013 21:38:40 -0800 Subject: [PATCH 4/5] Fix belongsTo relation --- lib/datasource.js | 26 ++++++++++++-------------- lib/relations.js | 2 +- package.json | 2 +- 3 files changed, 14 insertions(+), 16 deletions(-) diff --git a/lib/datasource.js b/lib/datasource.js index 62e1a688..0f0755a2 100644 --- a/lib/datasource.js +++ b/lib/datasource.js @@ -232,7 +232,7 @@ DataSource.prototype.setup = function(name, settings) { } connector = require(name); } catch (e) { - return console.log('\nWARNING: LoopbackData connector "' + name + '" is not installed,\nso your models would not work, to fix run:\n\n npm install ' + name, '\n'); + return console.log('\nWARNING: LoopBack connector "' + name + '" is not installed,\nso your models would not work, to fix run:\n\n npm install ' + name, '\n'); } } } @@ -920,7 +920,7 @@ DataSource.prototype.discoverSchemas = function (modelName, options, cb) { } var self = this; - var schemaName = this.name || this.connector.name; + var schemaName = this.connector.name || this.name; var tasks = [ this.discoverModelProperties.bind(this, modelName, options), @@ -1027,14 +1027,13 @@ DataSource.prototype.discoverSchemas = function (modelName, options, cb) { console.log('Foreign keys: ', fks); } + schema.options.relations = {}; foreignKeys.forEach(function (fk) { var propName = fromDBName(fk.pkTableName, true); - schema.properties[propName] = { - type: fromDBName(fk.pkTableName, false), - relation: { - type: 'belongsTo', - foreignKey: fromDBName(fk.pkColumnName, true) - } + schema.options.relations[propName] = { + model: fromDBName(fk.pkTableName, false), + type: 'belongsTo', + foreignKey: fromDBName(fk.fkColumnName, true) }; var key = fk.pkOwner + '.' + fk.pkTableName; @@ -1175,14 +1174,13 @@ DataSource.prototype.discoverSchemasSync = function (modelName, options) { console.log('Foreign keys: ', fks); } + schema.options.relations = {}; foreignKeys.forEach(function (fk) { var propName = fromDBName(fk.pkTableName, true); - schema.properties[propName] = { - type: fromDBName(fk.pkTableName, false), - relation: { - type: 'belongsTo', - foreignKey: fromDBName(fk.pkColumnName, true) - } + schema.options.relations[propName] = { + model: fromDBName(fk.pkTableName, false), + type: 'belongsTo', + foreignKey: fromDBName(fk.fkColumnName, true) }; var key = fk.pkOwner + '.' + fk.pkTableName; diff --git a/lib/relations.js b/lib/relations.js index 7cf2aaa9..a5b043f4 100644 --- a/lib/relations.js +++ b/lib/relations.js @@ -195,7 +195,7 @@ Relation.belongsTo = function (anotherClass, params) { } } - var idName = this.dataSource.idName(this.modelName) || 'id'; + var idName = this.dataSource.idName(anotherClass.modelName) || 'id'; var methodName = params.as || i8n.camelize(anotherClass.modelName, true); var fk = params.foreignKey || methodName + 'Id'; diff --git a/package.json b/package.json index 77ee23d5..34712bfc 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "loopback-datasource-juggler", - "version": "1.2.5", + "version": "1.2.6", "description": "LoopBack DataSoure Juggler", "keywords": [ "StrongLoop", From adc9482df0384e757971cfd3786ce0ccac46644f Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Thu, 5 Dec 2013 15:19:40 -0800 Subject: [PATCH 5/5] Enhance the test case with more assertions --- test/loopback-dl.test.js | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/test/loopback-dl.test.js b/test/loopback-dl.test.js index 62be6a4f..75c0115e 100644 --- a/test/loopback-dl.test.js +++ b/test/loopback-dl.test.js @@ -674,8 +674,8 @@ describe('DataSource constructor', function(){ }); -describe('Injected remotable methods', function(){ - it('are not shared across models', function() { +describe('Injected methods from connectors', function(){ + it('are not shared across models for remote methods', function() { var ds = new DataSource('memory'); var M1 = ds.createModel('M1'); var M2 = ds.createModel('M2'); @@ -686,9 +686,21 @@ describe('Injected remotable methods', function(){ M1.create.shared = false; assert.equal(M1.create.shared, false, 'M1.create should be local now'); assert.equal(M2.create.shared, true, 'M2.create should stay remotable'); - assert.notEqual(M1.prototype.save, M2.prototype.save, - 'non-remote methods are not shared'); }); + it('are not shared across models for non-remote methods', function() { + var ds = new DataSource('memory'); + var M1 = ds.createModel('M1'); + var M2 = ds.createModel('M2'); + var m1 = M1.prototype.save; + var m2 = M2.prototype.save; + assert.notEqual(m1, m2, 'non-remote methods are not shared'); + assert.equal(!!m1.shared, false, 'M1.save is not remotable'); + assert.equal(!!m2.shared, false, 'M2.save is not remotable'); + m1.shared = true; + assert.equal(m1.shared, true, 'M1.save is now remotable'); + assert.equal(!!m2.shared, false, 'M2.save is not remotable'); + + }); });